mach misses build warnings that are printed using relative paths (due to use of "--with-ccache" and CCACHE_BASEDIR)

NEW
Unassigned

Status

()

Core
Build Config
5 years ago
a month ago

People

(Reporter: dholbert, Unassigned)

Tracking

Trunk
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Created attachment 665726 [details]
build log (with "make -j1" and gcc/g++ 4.7.2)

"mach warnings-list" currently only detects 4 build warnings, in a full clobber build with gcc-4.7 / g++-4.7 on my Linux 64-bit laptop. (Ubuntu 12.10 b2, gcc 4.7.2)

(Probably not surprising, but just for the record: this is true even after I apply my hackaround from bug 794713, which lets the warnings actually get displayed in my terminal during the build process.  They're still not detected in mach's warning list after the build completes.)

At gps's request, I'm filing this bug and attaching a clobber build log (with make and "-j1") so we've got a record of the sorts of warnings gcc-4.7/g++-4.7 produces

Comment 1

5 years ago
Comment on attachment 665726 [details]
build log (with "make -j1" and gcc/g++ 4.7.2)

Ugh. Did I mention I hate relative source directory builds? They make it significantly more difficult to parse build logs because each line must be examined in context for the directory it appeared in. And, the only way to do this is by looking at make's "Entering directory" messages. This is problematic because A) these can be interleaved from multiple make processes running in parallel B) those strings are localized.

So, basically resolving the actual filename from relative source directory builds is impossible to do properly.

Now, the current compilation warnings database requires paths to be resolved to actual files because it stores the hash of the file triggering the warning. When warnings are inserted, if the hash of the file changed since the last insertion, we wipe all warnings associated with that file since they are no longer valid (they will presumably be re-inserted or will have disappeared). Anyway, this intelligence allows warnings recording to be smarter than just a log parser. I am quite inclined to keep this functionality because it offers compelling advantages.

Anyway, my opinion is we should outlaw relative source directory builds: srcdir and objdir should be defined in terms of absolute paths. But, that's outside the scope of this bug.

Can you resubmit a log from a build where absolute paths are used? I'm pretty sure all the relative path badness from this log will be replaced with absolute paths. But, I want to make sure. I may throw up a patch that I /think/ will work. We'll see how quickly this barrel aged stout kicks in...

Comment 2

5 years ago
And the code I wrote for this was *completely* identical to that for Clang's warning format!

I'm pretty sure we already parse GCC 4.7 warnings just fine. However, the warnings database does reject warnings it cannot map to a known file. And, since it doesn't know how to resolve relative paths, it cannot map to a known file. So, I'm pretty sure if you start using absolute paths, you'll find this will magically start working and this bug could be marked WORKSFORME. Although, there are certainly areas for improved messaging around relative paths, etc.
(Reporter)

Comment 3

5 years ago
Hmm... In a build where I never set MOZ_OBJDIR at all (so it's not "@TOPSRCDIR@/../obj"), it looks like most my build warnings are still printed with relative paths, e.g.:

353.44 In file included from ../../../js/src/jsfun.h:14:0,
353.44                  from ../../../js/src/vm/Stack.h:11,
353.44                  from ../../../js/src/jscntxt.h:33,
353.44                  from ../../../js/src/ion/x64/Bailouts-x64.cpp:8:
353.44 ../../../js/src/jsobj.h:1029:30: warning: inline function ‘js::StringObject& JSObject::asString()’ used but never defined [enabled by default]

so I suspect mach won't see them. Any idea how I can fix that?

Comment 4

5 years ago
What do the first few variables in objdir/Makefile look like? Mine are:

DEPTH           = .
topsrcdir       = /home/gps/src/mozilla-central
srcdir          = /home/gps/src/mozilla-central
VPATH           = /home/gps/src/mozilla-central

I suspect yours are relative paths.
(Reporter)

Comment 5

5 years ago
Nope, they're absolute.

DEPTH		= .
topsrcdir	= /scratch/work/builds/mozilla-central/mozilla-central.12-03-19.10-25/mozilla
srcdir		= /scratch/work/builds/mozilla-central/mozilla-central.12-03-19.10-25/mozilla
VPATH		= /scratch/work/builds/mozilla-central/mozilla-central.12-03-19.10-25/mozilla
(Reporter)

Updated

5 years ago
Summary: Make mach support parsing GCC 4.7 (g++-4.7) build warnings → mach misses build warnings that are, for some reason, printed using relative paths (using gcc-4.7 / g++-4.7)
(Reporter)

Comment 6

5 years ago
However, I was building from within a directory that was symbolically linked to $srcdir. I wonder if that affects things... I'll try building directly inside the absolute directory.
(Reporter)

Comment 7

5 years ago
Nope, that doesn't help.  I wonder if --with-ccache is triggering this, though... I seem to recall ccache working better with relative paths. Maybe we turn on something if you build --with-ccache.
(Reporter)

Comment 8

5 years ago
Yeah! If I remove --with-ccache, then I get absolute paths in my build warnings.
Summary: mach misses build warnings that are, for some reason, printed using relative paths (using gcc-4.7 / g++-4.7) → mach misses build warnings that are printed using relative paths due to use of "--with-ccache"

Comment 9

5 years ago
Under the hood ccache is doing abspath/realpath voodoo to try to normalize down to the actual files so it can increase its cache hit rate. So, paths may be altered by the time they hit the compiler.

You mentioned symlinks were involved. I bet your symlink is relative and not absolute and that changing it to absolute will cause things to work with ccache.

If that doesn't work, I bet blowing away your ccache cache will cause it to work.

I think we should fix this bug by exposing the error raised by the warnings database when it can't resolve the source file. The only reason it isn't exposed today is because in "proper" builds it is still failing when encountering warnings in NSS. If we fix that issue, we can expose all warnings. We could possibly combine this with logic to normalize the object directory to an absolute path. That should minimize issues with relative paths seen here.
(Reporter)

Comment 10

5 years ago
(In reply to Gregory Szorc [:gps] from comment #9)
> You mentioned symlinks were involved. I bet your symlink is relative and not
> absolute and that changing it to absolute will cause things to work with
> ccache.

(nope -- per comment 7k I don't think symlinks are involved, since I didn't get any improvement from building in the non-symlinked version of the directory)

> If that doesn't work, I bet blowing away your ccache cache will cause it to work.

I'm dubious... I think "--with-ccache" autoconfigures you to use relative paths (by doing "--with-srcdir=.." under the hood), and once that's enabled, I suspect we're hosed regardless of what's in the cache.
(Reporter)

Comment 11

5 years ago
(see http://robert.ocallahan.org/2005/01/sharing-binaries-across-multiple_21.html for why --srcdir=.. is a good idea for ccache builds.)  I suspect (but have no proof) that --with-ccache is enabling that option or something similar.
I personally hold the belief that unless you are performing clobber builds all the time or are building from multiple trees (and doing CCACHE_BASEDIR magic), ccache makes builds slower. So, I have it disabled on my machines. YMMV.

I'd still like to fix this issue, however...
(Reporter)

Comment 13

5 years ago
(In reply to Gregory Szorc [:gps] from comment #12)
> I personally hold the belief that unless you are performing clobber builds
> all the time or are building from multiple trees (and doing CCACHE_BASEDIR
> magic), ccache makes builds slower.

ccache actually saves loads of time if you use patch queues. In particular -- suppose I have this patch-stack applied:
 reftests.patch
 some-tweak-to-nsISupports-idl.patch
 wip-bugfix.patch

Now suppose I want to add a new test in reftests.patch, at the bottom of my queue. So I qpop to that point, perform my edits, and then qpush back to wip-bugfix.patch.

So, now I've un-applied and re-applied some-tweak-to-nsISupports-idl.patch.  This means my next build will rebuild the world, even though the code hasn't actually changed.  However, if I've got ccache, then that rebuild-the-world pass will be quite fast. 
 
> I'd still like to fix this issue, however...

Thanks :)
I really want to make this work. However, the solutions I'm seeing that deal with resolving relative paths are all ugly.

There is an option to not ensure files exist before inserting into the database. But, the usability suffers. You lose actual paths (obviously) and the ability to prune old warnings. So, the net result is IMO hardly better than just looking at the log output.

We could crawl the filesystem and do some kind of fuzzy matching on seen paths. This seems like it would be a bit expensive and prone to false positives.

I think the proper solution is to get GCC/ccache to spit out absolute paths. I /think/ we should be able to normalize paths to absolute inside of mach/mozbuild. But first, I need to know where relative paths are being introduced.

dholbert: can you provide as much information on your setup as possible? I want to see:

* General directory structure. Paths, symlinks, etc to source directories and object directories.
* Content of your .mozconfig
* The command(s) you use to build the tree, including the CWD
* The first few lines of output from the above (looking for debug info it usually prints)
* The command you use to run |mach build|, including CWD
* The first few lines of output from |mach build| (looking for debug info)

We'll tease this out...
I just tested this on my Ubuntu 12.04 VM with GCC 4.6 and ccache and I am getting absolute paths.

mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/objdir
mk_add_options MOZ_MAKE_FLAGS="-j10 -s -w"

ac_add_options --enable-application=browser
ac_add_options --enable-tests
ac_add_options --enable-optimize
ac_add_options --with-ccache=/usr/bin/ccache

$ ./mach build
...
72.29 /home/gps/src/mozilla-central/js/src/jsproxy.cpp: In member function ‘virtual bool ScriptedDirectProxyHandler::defineProperty(JSContext*, JSObject*, jsid, js::PropertyDescriptor*)’:
(Reporter)

Comment 16

5 years ago
Figured it out -- it turns out I have CCACHE_BASEDIR specified in my environment, and that's what's giving me relative paths.

From the ccache manpage:
> CCACHE_BASEDIR
>     If you set the environment variable CCACHE_BASEDIR to an absolute
>     path to a directory, ccache rewrites absolute paths into relative
>     paths before computing the hash that identifies the compilation,
>     but only for paths under the specified directory. See the discussion
>     under COMPILING IN DIFFERENT DIRECTORIES.

IIRC, this is required so that ccache can figure out that two different m-c srcdirs actually have the same contents -- so that if you've built in one srcdir, ccache will be able to significantly speed up a later build in the other srcdir.

(I'm happy to accept that it's a nonstandard build config and not a priority to support at the moment, though.)
(Reporter)

Updated

5 years ago
Summary: mach misses build warnings that are printed using relative paths due to use of "--with-ccache" → mach misses build warnings that are printed using relative paths (due to use of "--with-ccache" and CCACHE_BASEDIR)
You need to log in before you can comment on or make changes to this bug.