Closed
Bug 1146892
Opened 9 years ago
Closed 9 years ago
Investigate fixing mozharness' gittool/hgtool to work without setting env
Categories
(Release Engineering :: Applications: MozharnessCore, defect)
Release Engineering
Applications: MozharnessCore
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mshal, Assigned: mshal)
References
Details
Attachments
(1 file, 1 obsolete file)
6.39 KB,
patch
|
mshal
:
review+
mshal
:
checked-in+
|
Details | Diff | Splinter Review |
In bug 1132123 I had to jump through some hoops with the environment settings in order to get both hg and git checkouts working on Windows. They should probably work out of the box using the default environment. I think at least the following things are issues: 1) git.exe isn't in the Windows PATH 2) SYSTEMROOT needs to be in the environment for 'import random' to work (affects both hg and git IIRC) 3) If PROPERTIES_FILE is in the environment, it can try to use twig settings for non-mc checkouts, like this git checkout of gaia: 18:08:28 INFO - 2015-03-20 18:08:28,010 git init -q /builds/slave/map-linux32_g-0000000000000000/build/src/gaia 18:08:28 INFO - 2015-03-20 18:08:28,033 command: START 18:08:28 INFO - 2015-03-20 18:08:28,033 command: git fetch -q https://git.mozilla.org/releases/gaia.git +refs/heads/projects/maple:refs/remotes/origin/projects/maple 18:08:28 INFO - 2015-03-20 18:08:28,033 command: cwd: /builds/hg-shared/git.mozilla.org/releases%2Fgaia.git 18:08:28 INFO - 2015-03-20 18:08:28,033 command: output: 18:08:28 INFO - fatal: Couldn't find remote ref refs/heads/projects/maple It tries to add +refs/heads/projects/maple, which doesn't exist in gaia.git, so the command fails. The PROPERTIES_FILE logic should probably be smarter here.
Comment 1•9 years ago
|
||
git/hgtool assume that the stuff in $PROPERTIES_FILE is for the "primary" checkout, e.g. gecko in most cases. So yeah, there will be problems if this environment var is set for checkouts of other repos like gaia.
Assignee | ||
Comment 2•9 years ago
|
||
This should fix both git and hg to work without setting the environment beforehand. If any of the required env settings are missing, it'll add them into the env before calling the tools scripts. This doesn't fix the PROPERTIES_FILE issue (I'm not quite sure how it should be fixed), but since this fix means we don't have to pass in the build env anyway, I don't think it's a priority. try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=531250a4e6c1 Note the failure summary in the b2g desktop windows builds appear to be bug 1119905, not related to this change.
Attachment #8590254 -
Flags: review?(jlund)
Comment 3•9 years ago
|
||
Comment on attachment 8590254 [details] [diff] [review] bug1146892-git-hg Review of attachment 8590254 [details] [diff] [review]: ----------------------------------------------------------------- looks good :) it's too bad we are adding more env config items in the hg and gitool scripts rather than having things configured properly but as mentioned below, I don't think it's worth untangling the bag of snakes. ::: mozharness/base/vcs/gittool.py @@ +61,5 @@ > branch = c.get('branch') > clean = c.get('clean') > share_base = c.get('vcs_share_base', os.environ.get("GIT_SHARE_BASE_DIR", None)) > env = {'PATH': os.environ.get('PATH')} > env.update(c.get('env', {})) it be nice to set this correctly in every config like you are for configs/b2g/desktop_windows32.py. However I don't think it's worth blocking and testing everywhere. @@ +62,5 @@ > clean = c.get('clean') > share_base = c.get('vcs_share_base', os.environ.get("GIT_SHARE_BASE_DIR", None)) > env = {'PATH': os.environ.get('PATH')} > env.update(c.get('env', {})) > + if sys.platform == 'win32': can we use self._is_windows() ? ::: mozharness/base/vcs/hgtool.py @@ +74,5 @@ > if c.get('env'): > env.update(c['env']) > if share_base is not None: > env['HG_SHARE_BASE_DIR'] = share_base > + if sys.platform == 'win32': _is_windows() here too
Attachment #8590254 -
Flags: review?(jlund) → review+
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to Jordan Lund (:jlund) from comment #3) > @@ +62,5 @@ > > clean = c.get('clean') > > share_base = c.get('vcs_share_base', os.environ.get("GIT_SHARE_BASE_DIR", None)) > > env = {'PATH': os.environ.get('PATH')} > > env.update(c.get('env', {})) > > + if sys.platform == 'win32': > > can we use self._is_windows() ? Oh, I didn't know about that - it seems to work fine. Plus it gets rid of the 'import sys' addition.
Assignee | ||
Comment 5•9 years ago
|
||
Updated with _is_windows(); r+ carried forward.
Attachment #8590254 -
Attachment is obsolete: true
Attachment #8591066 -
Flags: review+
Assignee | ||
Comment 6•9 years ago
|
||
Comment on attachment 8591066 [details] [diff] [review] bug1146892-git-hg https://hg.mozilla.org/build/mozharness/rev/6d88c905b811
Attachment #8591066 -
Flags: checked-in+
Comment 7•9 years ago
|
||
mozharness production tag moved to: https://hg.mozilla.org/build/mozharness/rev/production
Assignee | ||
Updated•9 years ago
|
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•