Closed Bug 1390916 Opened 3 years ago Closed 3 years ago

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

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: mshal, Assigned: mshal)

References

Details

Attachments

(3 files)

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 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 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 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-
(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 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+
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
Depends on: 1416465
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.