The default bug view has changed. See this FAQ.

access violation: while compiling xulrunner tries to test for Mercurial repositories above its build dir

RESOLVED FIXED in mozilla1.9.3a1

Status

()

Core
Build Config
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: Arne Babenhauserheide, Assigned: Jory A. Pratt)

Tracking

Trunk
mozilla1.9.3a1
Other
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
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

Updated

8 years ago
Component: XUL → Build Config
QA Contact: xptoolkit.widgets → build-config
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.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → WONTFIX
(Reporter)

Comment 2

8 years ago
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.
(Reporter)

Comment 3

8 years ago
PS: I use a Gentoo system. Changing the source dir is nontrivial and systemwide.
Yes, I'm saying that your problem is such an edge case that I would not take a patch to fix it.
(Reporter)

Comment 5

8 years ago
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
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).
Status: RESOLVED → UNCONFIRMED
Resolution: WONTFIX → ---
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 7

8 years ago
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.
Attachment #405285 - Flags: review?(ted.mielczarek)
(Reporter)

Comment 8

8 years ago
@Gavin: Sorry for not answering - my mailbox got flooded and I seem to have overlooked your mail...
Attachment #405285 - Flags: review?(ted.mielczarek) → review+
Keywords: checkin-needed

Updated

8 years ago
Assignee: nobody → anarchy
Could you attach a hg patch?
(Assignee)

Comment 10

8 years ago
(In reply to comment #9)
> Could you attach a hg patch?

I will create an hg patch based on trunk as soon as possible.
http://hg.mozilla.org/mozilla-central/rev/1a1fbe3a6393
Status: NEW → RESOLVED
Last Resolved: 8 years ago8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
(Reporter)

Comment 12

8 years ago
many thanks!
Blocks: 524349
While there, I wonder why they use 2 '&&' and 1 ';':
shouldn't it be he same everywhere?
Flags: in-testsuite-
Version: unspecified → Trunk
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.
Attachment #405285 - Flags: approval1.9.2?
Attachment #405285 - Flags: approval1.9.1.4?
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
What version of hg is that? Does it print some error or help text if you run "hg identify ." in the sourcedir?
Comment on attachment 405285 [details] [diff] [review]
ensure we only look inside our src_dir and not rest of filesystem

Pushing out approval request.
Attachment #405285 - Flags: approval1.9.1.4? → approval1.9.1.5?
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.
Attachment #405285 - Flags: approval1.9.1.6? → approval1.9.1.6-
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.
Attachment #405285 - Flags: approval1.9.2?
Blocks: 515792
(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 ;->
You need to log in before you can comment on or make changes to this bug.