Closed Bug 517417 Opened 10 years ago Closed 10 years ago
access violation: while compiling xulrunner tries to test for Mercurial repositories above its build dir
User-Agent: Mozilla/5.0 (compatible; Konqueror/4.3; Linux) KHTML/4.3.1 (like Gecko) Build Identifier: xulrunner-126.96.36.199 I tried to build xulrunner on my Gentoo which creates a sandbox for it in /var/tmp/portage/net-libs/xulrunner-188.8.131.52/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
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
Closed: 10 years ago
Resolution: --- → WONTFIX
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.
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.
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 <email@example.com> # 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
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)
@Gavin: Sorry for not answering - my mailbox got flooded and I seem to have overlooked your mail...
Attachment #405285 - Flags: review?(ted.mielczarek) → review+
Could you attach a hg patch?
(In reply to comment #9) > Could you attach a hg patch? I will create an hg patch based on trunk as soon as possible.
Status: NEW → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
While there, I wonder why they use 2 '&&' and 1 ';': shouldn't it be he same everywhere?
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=?": "approval184.108.40.206=?": No risk, faster and safer.
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: approval220.127.116.11? → approval18.104.22.168?
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: approval22.214.171.124? → approval126.96.36.199-
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.
(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.