Closed Bug 1361172 Opened 3 years ago Closed 3 years ago

Intermittent Static Windows build error: Exception: ('failed to run any of the repo manifest commands', [['hg', 'manifest', '-q'], ['git', 'rev-parse', '--show-cdup']])

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox-esr52 fixed, firefox53 wontfix, firefox54 fixed, firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr52 --- fixed
firefox53 --- wontfix
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: gps)

Details

(Keywords: intermittent-failure, Whiteboard: [stockwell infra])

Attachments

(1 file)

Intermittent?!
this is increasing in frequency, :gps, can you take a look at this bug to reduce the impact is has on the trees?
Flags: needinfo?(gps)
Whiteboard: [stockwell infra]
Patch submitted for review.
Flags: needinfo?(gps)
Comment on attachment 8869261 [details]
Bug 1361172 - Rewrite code for finding files in VCS checkout;

https://reviewboard.mozilla.org/r/140824/#review144340

::: python/mozversioncontrol/mozversioncontrol/__init__.py:156
(Diff revision 1)
> +        elif os.path.isdir(os.path.join(path, '.git')):
> +            return GitRepository(path)

I suspect you'll want me to use `git rev-parse` instead of path traversal. But I'm too lazy to look up exactly what incantation is needed.
Comment on attachment 8869261 [details]
Bug 1361172 - Rewrite code for finding files in VCS checkout;

https://reviewboard.mozilla.org/r/140824/#review144342

After thinking about this a little more, I would not be opposed to abandoning the "find the repo" logic and replacing with "look at __file__/../.." in the 3 check_* scripts. That approach will be more robust and won't accidentally find non-Firefox repos.
Comment on attachment 8869261 [details]
Bug 1361172 - Rewrite code for finding files in VCS checkout;

https://reviewboard.mozilla.org/r/140824/#review144340

> I suspect you'll want me to use `git rev-parse` instead of path traversal. But I'm too lazy to look up exactly what incantation is needed.

git rev-parse --show-cdup, but I'm not 100% sure it wasn't broken in some cases at some point.
Comment on attachment 8869261 [details]
Bug 1361172 - Rewrite code for finding files in VCS checkout;

https://reviewboard.mozilla.org/r/140824/#review144520

::: python/mozversioncontrol/mozversioncontrol/__init__.py:108
(Diff revision 1)
>  
>      def forget_add_remove_files(self, path):
>          self._run('forget', path)
>  
> +    def get_files_in_working_directory(self):
> +        return self._run('files').splitlines()

You should use -0 and split on \0

::: python/mozversioncontrol/mozversioncontrol/__init__.py:129
(Diff revision 1)
>  
>      def forget_add_remove_files(self, path):
>          self._run('reset', path)
>  
> +    def get_files_in_working_directory(self):
> +        return self._run('ls-files', '--full-name').splitlines()

self._run uses the repository root directory as cwd, so you don't need --full-name, which only matters when run from a subdirectory. (FWIW, hg files has the same behavior as git ls-files when run from subdirectories, but afaict doesn't have an equivalent option)

You should use -z and split on \0.

::: python/mozversioncontrol/mozversioncontrol/__init__.py:156
(Diff revision 1)
> +                break
> +
> +    for path in ancestors(os.getcwd()):
> +        if os.path.isdir(os.path.join(path, '.hg')):
> +            return HgRepository(path)
> +        elif os.path.isdir(os.path.join(path, '.git')):

You've got yourself bug 1313446 in that new code, while the fix for that bug is a few lines above ;)

Why not simply do:

for path in ancestors(os.getcwd()):
    try:
        return get_repository_object(path)
    (...)
        

Also, considering bug 1339240, we should use this module in other places. I found at least python/mozbuild/mozbuild/base.py and python/mozboot/mozboot/bootstrap.py.
Attachment #8869261 - Flags: review?(mh+mozilla)
Comment on attachment 8869261 [details]
Bug 1361172 - Rewrite code for finding files in VCS checkout;

https://reviewboard.mozilla.org/r/140824/#review145300

::: python/mozversioncontrol/mozversioncontrol/__init__.py:166
(Diff revision 2)
> +        try:
> +            return get_repository_object(path)
> +        except InvalidRepoPath:
> +            continue
> +
> +    raise Exception('Could not find Mercurial or Git checkout from %s' %

s/from/for/ ?
Attachment #8869261 - Flags: review?(mh+mozilla) → review+
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/965639b40b6a
Rewrite code for finding files in VCS checkout; r=glandium
https://hg.mozilla.org/mozilla-central/rev/965639b40b6a
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Assignee: nobody → gps
Component: Build Config → General
Product: Firefox → Firefox Build System
Target Milestone: Firefox 55 → mozilla55
You need to log in before you can comment on or make changes to this bug.