Process install manifests with --track in the recursive make backend

RESOLVED FIXED in Firefox 57

Status

Firefox Build System
General
RESOLVED FIXED
8 months ago
2 months ago

People

(Reporter: mshal, Assigned: mshal)

Tracking

unspecified
mozilla57

Firefox Tracking Flags

(firefox57 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Assignee)

Description

8 months ago
While looking into https://bugzilla.mozilla.org/show_bug.cgi?id=902825#c48, I needed to use the newer --track style of install manifests instead of the older --no-remove style. This should be useful regardless of the outcome of bug 902825 since it provides finer grained tracking of installed files.

For example, removing [test_client.js] from services/fxaccounts/tests/xpcshell/xpcshell.ini and running the install-test-files target will not correctly remove test_client.js from the objdir/_tests/ directory unless we use --track.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 4

8 months ago
mozreview-review
Comment on attachment 8898336 [details]
Bug 1390916 - Make .PHONY rule match the actual make targets;

https://reviewboard.mozilla.org/r/169686/#review175360
Attachment #8898336 - Flags: review?(gps) → review+

Comment 5

8 months ago
mozreview-review
Comment on attachment 8898337 [details]
Bug 1390916 - Remove references to dist/sdk install manifest;

https://reviewboard.mozilla.org/r/169688/#review175362
Attachment #8898337 - Flags: review?(gps) → review+

Comment 6

8 months ago
mozreview-review
Comment on attachment 8898338 [details]
Bug 1390916 - Always use --track instead of --no-remove for install manifests;

https://reviewboard.mozilla.org/r/169690/#review175372

Please ask for re-review once you've replied to my question on changing the default behavior for directory symlinks and empty directories.

::: python/mozbuild/mozbuild/action/process_install_manifest.py:63
(Diff revision 1)
> -        remove_all_directory_symlinks=remove_all_directory_symlinks,
> -        remove_empty_directories=remove_empty_directories)
> +        remove_all_directory_symlinks=False,
> +        remove_empty_directories=False)

The default behavior is changed here. Is this intentional? If so, could you please rationalize why the new behavior is proper?
Attachment #8898338 - Flags: review?(gps) → review-
(Assignee)

Comment 7

8 months ago
(In reply to Gregory Szorc [:gps] from comment #6)
> ::: python/mozbuild/mozbuild/action/process_install_manifest.py:63
> (Diff revision 1)
> > -        remove_all_directory_symlinks=remove_all_directory_symlinks,
> > -        remove_empty_directories=remove_empty_directories)
> > +        remove_all_directory_symlinks=False,
> > +        remove_empty_directories=False)
> 
> The default behavior is changed here. Is this intentional? If so, could you
> please rationalize why the new behavior is proper?

Good catch - I think I misinterpreted how these flags were used by copier.py in the case where we have an existing track file. It looks like we do need them to be True in that case in order to correctly remove a directory when files are no longer installed there. I'll update the patch...
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 11

8 months ago
mozreview-review
Comment on attachment 8898338 [details]
Bug 1390916 - Always use --track instead of --no-remove for install manifests;

https://reviewboard.mozilla.org/r/169690/#review176054
Attachment #8898338 - Flags: review?(gps) → review+

Comment 12

8 months ago
Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7c9a5812fc16
Make .PHONY rule match the actual make targets; r=gps
https://hg.mozilla.org/integration/autoland/rev/1b502238070f
Remove references to dist/sdk install manifest; r=gps
https://hg.mozilla.org/integration/autoland/rev/3a181cd6052b
Always use --track instead of --no-remove for install manifests; r=gps
https://hg.mozilla.org/mozilla-central/rev/7c9a5812fc16
https://hg.mozilla.org/mozilla-central/rev/1b502238070f
https://hg.mozilla.org/mozilla-central/rev/3a181cd6052b
Status: NEW → RESOLVED
Last Resolved: 8 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57

Updated

5 months ago
Depends on: 1416465

Updated

2 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.