On 11/27/13 9:04 PM, Nico Kadel-Garcia wrote:
> * I alphabetized the arguments and the subroutines for consistency and
Let's stick with the functional changes first and worry about the consistency
changes after. Combining them just makes it harder to review the functional
> * You mentioned that I should not use "$()" syntax. It was helpful to
> get the "printf" for SQLITE_VERSION handling into a single line,
> because printf parsing is a bit odd.. I'm startled if any system
> modern enough to compose Subversion 1.8.5';s dependencies do not
> support that syntax, but but it's easy to unroll that change.
Solaris's /bin/sh doesn't support $() unfortunately. Yes $() is part of POSIX.
We've had users complain about this not working. Unfortunately that means you
have to split that onto 2 lines. I wasn't thrilled about doing that but
couldn't find a portable way to do it in one line.
> * You point out that gtest is not yet used. If nothing uses it, why
> was it ever added to get-deps.sh? I can disable that: I started
> writing my version of this before danielsh pulled out the automatic
> gtest, and hadn't merged back that modification.
We don't always remove work in progress changes that have no impact on end
users. The gtest dependency was added for some work on a C++ binding, which
isn't anywhere close to usable and isn't advertised to anyone.
> * The "return 1" everywhere is to *stop* the individual subroutine,
> but allow it to continue on to the next download. If I see one more
> routine that does a "cd" or "wget"and ignores the result and continues
> merrily on its way, I'm going to get upset. That said, the handling of
> bad command lines is a bit odd, but I didn't elect to try to rewrite
Agreed about checking results. return without the 1 feels more natural. For
one thing that should preserve the exit value of the last command executed in
> * I updated the SQLite autoconf tool because the www.sqlite.org
> doesn't list the release in the existing get-deps.sh as still being
> supported. The repository is not browseable, so the only way to find
> that old version is to know that it already exists. That strikes me as
> a recipe for pain, so I grabbed the more resent release.
We need to be really careful about updating sqlite. The version you updated to
may be fine. But I know for instance that you don't want to move to a 3.8.x
version. 3.8.x has some poor performance with our wc databases.
I don't really see the problem with us downloading a file not listed on their
website. Their download links seem to be stable, zlib on the other is a much
bigger problem in this respect because they remove the download link we're
using if the version isn't the current version anymore.
Received on 2013-11-28 07:33:32 CET