1365722 - Cleanup 'mach repackage' for windows work

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Michael Shal [:mshal]]

While working on bug 1360525, I came across a few cleanup things that we need to sort out before adding a new repackaging type. I split this work out into a separate bug so that it can be reviewed now, since this part will likely not change during integration testing.

Comment 1

[Michael Shal [:mshal]]

Attachment: Bug 1365722 - Move repackage_dmg into a repackaging/ directory;

It makes more sense to have the repackage commands in a separate
directory, since Windows repackaging will add several new types.

Review commit: https://reviewboard.mozilla.org/r/140336/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/140336/

Comment 2

[Michael Shal [:mshal]]

Attachment: Bug 1365722 - Rename 7z_exe_* to exe_7z_*;

Python can run these files with 'python -m 7z_exe_foo', but using
'import 7z_exe_foo' is not allowed because the module begins with a
number.

Review commit: https://reviewboard.mozilla.org/r/140338/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/140338/

Comment 3

[Michael Shal [:mshal]]

Attachment: Bug 1365722 - Use correct permission bits in chmod;

The chmod permissions need to be in octal format to get the expected
permissions settings. This only seems to affect the output if we run the
repackaging command on Linux, but it should still be fixed.

Review commit: https://reviewboard.mozilla.org/r/140340/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/140340/

Comment 4

[Michael Shal [:mshal]]

Attachment: Bug 1365722 - Move application.ini processing out to a separate file;

Windows repackaging for complete mar files will also need to pull a
different value out of the application.ini file, so this code should be
shareable.

Review commit: https://reviewboard.mozilla.org/r/140342/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/140342/

Comment 5

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8868740 [details]
Bug 1365722 - Move repackage_dmg into a repackaging/ directory;

https://reviewboard.mozilla.org/r/140336/#review144296

Comment 6

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8868741 [details]
Bug 1365722 - Rename 7z_exe_* to exe_7z_*;

https://reviewboard.mozilla.org/r/140338/#review144298

::: commit-message-fda08:5
(Diff revision 1)
> +Bug 1365722 - Rename 7z_exe_* to exe_7z_*; r?chmanchester
> +
> +Python can run these files with 'python -m 7z_exe_foo', but using
> +'import 7z_exe_foo' is not allowed because the module begins with a
> +number.

That's extraordinary. Do you have a reference to the CPython code or issue we can include here?

Comment 7

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8868742 [details]
Bug 1365722 - Use correct permission bits in chmod;

https://reviewboard.mozilla.org/r/140340/#review144300

Comment 8

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8868743 [details]
Bug 1365722 - Move application.ini processing out to a separate file;

https://reviewboard.mozilla.org/r/140342/#review144302

Comment 9

[Michael Shal [:mshal]]

(In reply to Chris Manchester (Offline Friday May 19th) from comment #6)
> Comment on attachment 8868741 [details]
> > +Python can run these files with 'python -m 7z_exe_foo', but using
> > +'import 7z_exe_foo' is not allowed because the module begins with a
> > +number.
> 
> That's extraordinary. Do you have a reference to the CPython code or issue
> we can include here?

I haven't found anything official, but I think it's because identifiers have to start with a letter or underscore [1]. So something like this doesn't parse correctly since you can't name a variable "2foo":

import 2foo
2foo.foo()

But if you use the -m syntax, I believe it ends up calling something like import_module() directly, which takes a string, so it ends up skipping the parsing that would normally happen when using import in a python file.

[1] https://stackoverflow.com/a/17487228

Comment 10

[Michael Shal [:mshal]]

Comment on attachment 8868741 [details]
Bug 1365722 - Rename 7z_exe_* to exe_7z_*;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/140338/diff/1-2/

Comment 11

[Michael Shal [:mshal]]

Comment on attachment 8868742 [details]
Bug 1365722 - Use correct permission bits in chmod;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/140340/diff/1-2/

Comment 12

[Michael Shal [:mshal]]

Comment on attachment 8868743 [details]
Bug 1365722 - Move application.ini processing out to a separate file;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/140342/diff/1-2/

Comment 13

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 8ddbd094b5b9 -d df65e023bd5a: rebasing 397584:8ddbd094b5b9 "Bug 1365722 - Move repackage_dmg into a repackaging/ directory; r=cmanchester+432261"
rebasing 397585:f4ac8775c6e4 "Bug 1365722 - Rename 7z_exe_* to exe_7z_*; r=cmanchester+432261"
merging toolkit/mozapps/installer/upload-files.mk
warning: conflicts while merging toolkit/mozapps/installer/upload-files.mk! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 14

[Michael Shal [:mshal]]

Comment on attachment 8868740 [details]
Bug 1365722 - Move repackage_dmg into a repackaging/ directory;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/140336/diff/1-2/

Comment 15

[Michael Shal [:mshal]]

Comment on attachment 8868741 [details]
Bug 1365722 - Rename 7z_exe_* to exe_7z_*;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/140338/diff/2-3/

Comment 16

[Michael Shal [:mshal]]

Comment on attachment 8868742 [details]
Bug 1365722 - Use correct permission bits in chmod;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/140340/diff/2-3/

Comment 17

[Michael Shal [:mshal]]

Comment on attachment 8868743 [details]
Bug 1365722 - Move application.ini processing out to a separate file;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/140342/diff/2-3/

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/95754c0a54fd
Move repackage_dmg into a repackaging/ directory; r=cmanchester+432261
https://hg.mozilla.org/integration/autoland/rev/ff556acd0782
Rename 7z_exe_* to exe_7z_*; r=cmanchester+432261
https://hg.mozilla.org/integration/autoland/rev/bb9a212e0bdf
Use correct permission bits in chmod; r=cmanchester+432261
https://hg.mozilla.org/integration/autoland/rev/4cfff4a6f66e
Move application.ini processing out to a separate file; r=cmanchester+432261

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/95754c0a54fd
https://hg.mozilla.org/mozilla-central/rev/ff556acd0782
https://hg.mozilla.org/mozilla-central/rev/bb9a212e0bdf
https://hg.mozilla.org/mozilla-central/rev/4cfff4a6f66e