Last Comment Bug 517417 - access violation: while compiling xulrunner tries to test for Mercurial repositories above its build dir
: access violation: while compiling xulrunner tries to test for Mercurial repos...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: Other Linux
: -- normal (vote)
: mozilla1.9.3a1
Assigned To: Jory A. Pratt
:
Mentors:
http://bugs.gentoo.org/show_bug.cgi?i...
Depends on:
Blocks: 515792 524349
  Show dependency treegraph
 
Reported: 2009-09-18 00:13 PDT by Arne Babenhauserheide
Modified: 2009-12-01 18:27 PST (History)
8 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
ensure we only look inside our src_dir and not rest of filesystem (1.98 KB, patch)
2009-10-08 10:20 PDT, Jory A. Pratt
ted: review+
dveditz: approval1.9.1.6-
Details | Diff | Splinter Review

Description Arne Babenhauserheide 2009-09-18 00:13:37 PDT
User-Agent:       Mozilla/5.0 (compatible; Konqueror/4.3; Linux) KHTML/4.3.1 (like Gecko)
Build Identifier: xulrunner-1.9.1.3

I tried to build xulrunner on my Gentoo which creates a sandbox for it in /var/tmp/portage/net-libs/xulrunner-1.9.1.3/work/ 

Write access outside this sandbox is not allowed (else it gets an access violation). 

Now I have a Mercurial repository in /var (to version some important files like my mpd playlists) and xulrunners check for a Mercurial repository seems to propagate outside the sandbox and try to write in my /var/.hg dir: 

...
F: open_wr
S: deny   
P: /var/.hg/wlock
A: /var/.hg/wlock
R: /var/.hg/wlock
C: /usr/bin/python2.6 /usr/bin/hg identify 
...
F: open_wr
S: deny   
P: /var/nxTE_D
A: /var/nxTE_D
R: /var/nxTE_D
C: /usr/bin/python2.6 /usr/bin/hg identify 



Reproducible: Always



Expected Results:  
The check for the hg repo shouldn't propagate outside the build dir to avoid that sandbox violation. 

I verified that the hg repo was the reason by moving it away before building. When there wasn't a hg repo in /var/, building worked without problems. 

Additional information is available in the according Gentoo bug: http://bugs.gentoo.org/show_bug.cgi?id=284673
Comment 1 Ted Mielczarek [:ted.mielczarek] 2009-09-18 11:01:02 PDT
We run "hg -R $topsrcdir identify" to find the changeset you're building from. I guess hg will look for a repo in a higher directory, and fail if that's readonly. I wouldn't take a patch for this. Put your source directory somewhere else.
Comment 2 Arne Babenhauserheide 2009-09-18 11:17:01 PDT
Does that mean I'll have to patch it myself to only do that check if $topsrcdir/.hg exists? 

It would simply mean to use 

if [ -d "$topsrcdir/.hg" ]; then hg -R $topsrcdir id; fi

instead of the simple hg id, since you know after all, where the .hg *must* be.
Comment 3 Arne Babenhauserheide 2009-09-18 11:17:44 PDT
PS: I use a Gentoo system. Changing the source dir is nontrivial and systemwide.
Comment 4 Ted Mielczarek [:ted.mielczarek] 2009-09-18 11:20:22 PDT
Yes, I'm saying that your problem is such an edge case that I would not take a patch to fix it.
Comment 5 Arne Babenhauserheide 2009-09-18 14:16:10 PDT
I know that you won't (easily?) take the patch, but since a clean fix (without if...fi) changes only 6 characters: 

You run "cd $(topsrcdir) ; hg identify" while the way which would work with higher up hg repos is to call either "cd $(topsrcdir) ; hg identify ." or "hg identify $(topsrcdir)". 

You do that in crashreporter/tools/symbolstore.py

There you call "hg id -i srcdir" which makes sure that other hg dirs can't interfere. Also it is a lot faster when there is no hg repo than letting hg traverse the filesystem tree (on my system the inexact way takes about 3 seconds when there is no hg repo above the srcdir. The exact way takes less than 300ms). 

You also pass the path in browser/locales/Makefile.in

So this fix unifies the way you identify the revision. 

It is either for you, or I'll ask the Gentoo devs if they'll take it. If you decide that my reasoning is good (and the 6 character change is warranted for a edge case), you can simply import the patch via "hg import". It's done in the 1.9.1 branch. 

# HG changeset patch                                                  
# User Arne Babenhauserheide <bab@draketo.de>                         
# Date 1253306010 -7200                                               
# Node ID be4db1d5647cbee6ea6bda8ff2e5992db16f1773                    
# Parent  71163cab3efb1d098ad0e0ac832e40b7a7d54ac4                    
Bug 517417.  Don't let hg identify traverse into higher directories.  

diff --git a/browser/app/Makefile.in b/browser/app/Makefile.in
--- a/browser/app/Makefile.in                                 
+++ b/browser/app/Makefile.in                                 
@@ -72,7 +72,7 @@ GRE_BUILDID = $(shell $(PYTHON) $(topsrc    
                                                              
 DEFINES += -DGRE_MILESTONE=$(GRE_MILESTONE) -DGRE_BUILDID=$(GRE_BUILDID)

-SOURCE_STAMP := $(shell cd $(topsrcdir) ; hg identify 2>/dev/null | cut -f1 -d' ')
+SOURCE_STAMP := $(shell cd $(topsrcdir) ; hg identify . 2>/dev/null | cut -f1 -d' ')
 ifdef SOURCE_STAMP
 DEFINES += -DMOZ_SOURCE_STAMP="$(SOURCE_STAMP)"
 endif
diff --git a/toolkit/content/Makefile.in b/toolkit/content/Makefile.in
--- a/toolkit/content/Makefile.in
+++ b/toolkit/content/Makefile.in
@@ -58,7 +58,7 @@ DEFINES += \
   -DCPPFLAGS="$(CPPFLAGS)" \
   $(NULL)

-CHANGESET := $(shell cd $(topsrcdir) && hg identify 2>/dev/null | cut -f1 -d' ')
+CHANGESET := $(shell cd $(topsrcdir) && hg identify . 2>/dev/null | cut -f1 -d' ')
 ifdef CHANGESET
 DEFINES += -DSOURCE_CHANGESET="$(CHANGESET)"
 endif
diff --git a/toolkit/xre/Makefile.in b/toolkit/xre/Makefile.in
--- a/toolkit/xre/Makefile.in
+++ b/toolkit/xre/Makefile.in
@@ -250,7 +250,7 @@ DEFINES     += -DHAVE_USR_LIB64_DIR
 endif
 endif

-SOURCE_STAMP := $(shell cd $(topsrcdir) && hg identify 2>/dev/null | cut -f1 -d' ')
+SOURCE_STAMP := $(shell cd $(topsrcdir) && hg identify . 2>/dev/null | cut -f1 -d' ')
 # strip a trailing slash from the repo URL because it's not always present,
 # and we want to construct a working URL in buildconfig.html
 # make+shell+sed = awful
Comment 6 :Gavin Sharp [email: gavin@gavinsharp.com] 2009-09-25 13:17:57 PDT
That patch looks reasonable. Have you tested to ensure that it works (i.e. about:buildconfig shows the right revision)?

You should attach it as an attachment and request review (set the review flag to "?" and enter ted's email address).
Comment 7 Jory A. Pratt 2009-10-08 10:20:09 PDT
Created attachment 405285 [details] [diff] [review]
ensure we only look inside our src_dir and not rest of filesystem

Exact same patch as in bug report, just attached and added to bug for review and inclusion. Patch has been tested and causes no danger.
Comment 8 Arne Babenhauserheide 2009-10-08 10:51:46 PDT
@Gavin: Sorry for not answering - my mailbox got flooded and I seem to have overlooked your mail...
Comment 9 Serge Gautherie (:sgautherie) 2009-10-20 15:12:54 PDT
Could you attach a hg patch?
Comment 10 Jory A. Pratt 2009-10-20 17:07:35 PDT
(In reply to comment #9)
> Could you attach a hg patch?

I will create an hg patch based on trunk as soon as possible.
Comment 11 Dão Gottwald [:dao] 2009-10-25 02:28:05 PDT
http://hg.mozilla.org/mozilla-central/rev/1a1fbe3a6393
Comment 12 Arne Babenhauserheide 2009-10-25 02:33:01 PDT
many thanks!
Comment 13 Serge Gautherie (:sgautherie) 2009-10-25 06:02:03 PDT
While there, I wonder why they use 2 '&&' and 1 ';':
shouldn't it be he same everywhere?
Comment 14 Serge Gautherie (:sgautherie) 2009-10-25 06:06:05 PDT
Comment on attachment 405285 [details] [diff] [review]
ensure we only look inside our src_dir and not rest of filesystem

"approval1.9.2=?":
"approval1.9.1.4=?":
No risk, faster and safer.
Comment 15 Doug Turner (:dougt) 2009-10-25 23:03:18 PDT
This change broke my Fennec / Maemo build.  Not sure why tinderbox's are still building.


/usr/bin/python2.5 /home/dougt/builds/mobile/mozilla-central/toolkit/xre/make-platformini.py --buildid=20091025215535 --sourcestamp=hg  print        aliases: /home/dougt/builds/mobile/mozilla-central/config/milestone.txt > platform.ini
Traceback (most recent call last):
  File "/home/dougt/builds/mobile/mozilla-central/toolkit/xre/make-platformini.py", line 24, in <module>
    (milestoneFile,) = args
ValueError: too many values to unpack
Comment 16 Ted Mielczarek [:ted.mielczarek] 2009-10-26 04:45:16 PDT
What version of hg is that? Does it print some error or help text if you run "hg identify ." in the sourcedir?
Comment 17 Samuel Sidler (old account; do not CC) 2009-10-28 12:56:40 PDT
Comment on attachment 405285 [details] [diff] [review]
ensure we only look inside our src_dir and not rest of filesystem

Pushing out approval request.
Comment 18 Daniel Veditz [:dveditz] 2009-11-04 16:34:10 PST
Comment on attachment 405285 [details] [diff] [review]
ensure we only look inside our src_dir and not rest of filesystem

Doesn't seem like the kind of fix we have to take on the stable branch. 1.9.1 approval denied.
Comment 19 :Gavin Sharp [email: gavin@gavinsharp.com] 2009-12-01 17:36:59 PST
Comment on attachment 405285 [details] [diff] [review]
ensure we only look inside our src_dir and not rest of filesystem

I think we'd rather take bug 515792 than just this bug, but probably not worth bothering for 1.9.2 at this point.
Comment 20 Serge Gautherie (:sgautherie) 2009-12-01 18:27:19 PST
(In reply to comment #13)
> While there, I wonder why they use 2 '&&' and 1 ';':
> shouldn't it be he same everywhere?

Bug 515792 will get rid of that too ;->

Note You need to log in before you can comment on or make changes to this bug.