Replace "Hammer" references with "Debug" as appropriate.

Replace "Hammer" references with "Debug" as appropriate.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=12849

[sgk, 2009-03-26 22:02:03]

Here are (again, untested) changes to the Hammer references in the source tree
proper for preliminary perusal.

[Mark Mentovai, 2009-03-26 22:15:56]

It's my understanding that in the current world, the SCons/Hammer build always
puts output into Hammer, whether it's debug-mode or release-mode.  Therefore, in
the current world, everything works for both debug and release when it
hard-codes Hammer.

When moving to sconsbuild/{Debug,Release}, we should ensure that everything
continues to work.  Hard-coding Debug might cause serious problems on a release
buildbot when it hits chrome_tests, for example.

For the things that are comments, it's probably enough to just write
{Debug,Release} or something in place of the existing Hammer to make this
obvious.  There are a couple of other things here that do look suspicious,
though.  Do you have a plan for handling them?

http://codereview.chromium.org/53118/diff/1/3
File third_party/libxslt/linux/xslt-config (right):

http://codereview.chromium.org/53118/diff/1/3#newcode92
Line 92: the_libs="-L${libdir} -lxslt
-L/home/mmoss/src/chrome/src/sconsbuild/Debug/third_party/libxml/scons
-L/usr/local/lib -lxml2 -lz -lm -lm"
Why are we hardcoding mmoss' build directory here?

Is Debug right?  What if whoever is running this built release?

Why -lm twice?

Should this maybe be using pkg-config?

Do we even need for this script to be checked in?  I have my doubts.

http://codereview.chromium.org/53118/diff/1/5
File tools/valgrind/chrome_tests.py (right):

http://codereview.chromium.org/53118/diff/1/5#newcode101
Line 101: dir_chrome = os.path.join(self._source_dir, "sconsbuild", "Debug")
I would suggest that everywhere you wrote "Debug" should be able to handle Debug
or Release as appropriate.  Not sure if you intend that for a future change,
though.

[Nicolas Sylvain, 2009-03-26 22:29:19]

http://codereview.chromium.org/53118/diff/1/8
File chrome/installer/linux/debian/build.sh (right):

http://codereview.chromium.org/53118/diff/1/8#newcode316
Line 316: BUILDDIR=$(readlink -f "${SCRIPTDIR}/../../../../sconsbuild/Debug")
would that work in release?

http://codereview.chromium.org/53118/diff/1/3
File third_party/libxslt/linux/xslt-config (right):

http://codereview.chromium.org/53118/diff/1/3#newcode92
Line 92: the_libs="-L${libdir} -lxslt
-L/home/mmoss/src/chrome/src/sconsbuild/Debug/third_party/libxml/scons
-L/usr/local/lib -lxml2 -lz -lm -lm"
i dont understand this file. I'm not reviewing it.

http://codereview.chromium.org/53118/diff/1/5
File tools/valgrind/chrome_tests.py (right):

http://codereview.chromium.org/53118/diff/1/5#newcode101
Line 101: dir_chrome = os.path.join(self._source_dir, "sconsbuild", "Debug")
instead of "debug" here you might be able to use the last tag in build_dir  (it
ends with debug or release)

[Michael Moss, 2009-03-26 22:59:00]

http://codereview.chromium.org/53118/diff/1/8
File chrome/installer/linux/debian/build.sh (right):

http://codereview.chromium.org/53118/diff/1/8#newcode316
Line 316: BUILDDIR=$(readlink -f "${SCRIPTDIR}/../../../../sconsbuild/Debug")
On 2009/03/26 22:29:19, Nicolas Sylvain wrote:
> would that work in release?

This is just the default. The buildbots should pass in the correct directory
with the -b option.

http://codereview.chromium.org/53118/diff/1/3
File third_party/libxslt/linux/xslt-config (right):

http://codereview.chromium.org/53118/diff/1/3#newcode92
Line 92: the_libs="-L${libdir} -lxslt
-L/home/mmoss/src/chrome/src/sconsbuild/Debug/third_party/libxml/scons
-L/usr/local/lib -lxml2 -lz -lm -lm"
On 2009/03/26 22:15:56, Mark Mentovai wrote:
> Why are we hardcoding mmoss' build directory here?
> 
> Is Debug right?  What if whoever is running this built release?

Ugh, what a mess. Who the hell let me check it in like this :) If we even still
need this, can we change those paths to use some BUILD_TARGET_DIR environment
variable instead of the hard-coded path?

[sgk, 2009-03-27 01:20:27]

http://codereview.chromium.org/53118/diff/1/8
File chrome/installer/linux/debian/build.sh (right):

http://codereview.chromium.org/53118/diff/1/8#newcode316
Line 316: BUILDDIR=$(readlink -f "${SCRIPTDIR}/../../../../sconsbuild/Debug")
On 2009/03/26 22:59:01, mmoss wrote:
> On 2009/03/26 22:29:19, Nicolas Sylvain wrote:
> > would that work in release?
> 
> This is just the default. The buildbots should pass in the correct directory
> with the -b option.

I'm changing the default here to 'Release'.  That seems to make more sense for
what a real distributed package would prefer, and as you say, our buildbots will
pass in the real location with -b.

http://codereview.chromium.org/53118/diff/1/3
File third_party/libxslt/linux/xslt-config (right):

http://codereview.chromium.org/53118/diff/1/3#newcode92
Line 92: the_libs="-L${libdir} -lxslt
-L/home/mmoss/src/chrome/src/sconsbuild/Debug/third_party/libxml/scons
-L/usr/local/lib -lxml2 -lz -lm -lm"
On 2009/03/26 22:59:01, mmoss wrote:
> On 2009/03/26 22:15:56, Mark Mentovai wrote:
> > Why are we hardcoding mmoss' build directory here?
> > 
> > Is Debug right?  What if whoever is running this built release?
> 
> Ugh, what a mess. Who the hell let me check it in like this :) If we even
still
> need this, can we change those paths to use some BUILD_TARGET_DIR environment
> variable instead of the hard-coded path?

Looking around a bit more, I don't think we need this file at all anymore.  It's
evidently the copy that was used to generate the linux/config.h file that was
checked in, and it looks like we don't actually use this in the gyp build
process.  I'll verify and remove.

http://codereview.chromium.org/53118/diff/1/5
File tools/valgrind/chrome_tests.py (right):

http://codereview.chromium.org/53118/diff/1/5#newcode101
Line 101: dir_chrome = os.path.join(self._source_dir, "sconsbuild", "Debug")
On 2009/03/26 22:29:19, Nicolas Sylvain wrote:
> instead of "debug" here you might be able to use the last tag in build_dir 
(it
> ends with debug or release)

Problem is that this is choosing a default if build_dir isn't set (i.e., no
--build-dir option is specified).

Looks like buildbot always passes in --build-dir.  After checking with erg, I'm
going to change this to error out if build_dir isn't specified, since the script
can't really know whether you want Debug or Release in the general case.

That would only break people who are running it by hand and not specifying
--build-dir.  I don't know if anyone is doing that on Linux, but we can fix any
real cases.

If there are people running valgrind tests by hand on Linux, and we therefore
want to pick something friendly for the default, 'Debug' seems better since that
more likely matches what most developers will be building in their trees.

[sgk, 2009-03-31 08:37:05]

TBR: mark,nsylvain,mmoss,maruel

This should fix the archive_build step, and either fix the red tests or point to
the next problem.