1743233 - ./mach repackage msix fails to infer parameters

Status: VERIFIED FIXED

(Firefox Build System :: General, defect)

Description

[Masatoshi Kimura [:emk]]

$ ./mach repackage msix
Error running mach:

    ['repackage', 'msix']

The error occurred in the implementation of the invoked mach command.

This should never occur and is likely a bug in the implementation of that
command.
You can invoke |./mach busted| to check if this issue is already on file. If it
isn't, please use |./mach busted file repackage| to report it. If |./mach busted| is
misbehaving, you can also inspect the dependencies of bug 1543241.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

AttributeError: 'repackage' object has no attribute 'get'

  File "f:\m\mozilla-central\python/mozbuild/mozbuild/mach_commands.py", line 2214, in repackage_msix
    if os.path.exists(command_context.get("bindir")):

$

Comment 1

[Mitchell Hentges [:mhentges] 🦀]

The command_context.get(...) lines are new as of bug 1742998, so I believe it's regressed by this patch.

Comment 2

[Masatoshi Kimura [:emk]]

I hesitated to say that this is a regression because ./mach repackage msix did not guess parameters at all before bug 1742998.

Comment 3

[Mitchell Hentges [:mhentges] 🦀]

I hesitated to say that this is a regression because ./mach repackage msix did not guess parameters at all before bug 1742998.

That's fair, but let's represent that other issue as a separate bug.

Comment 4

[[e-mail] (:bhearsum)]

(In reply to Masatoshi Kimura [:emk] from comment #2)

I hesitated to say that this is a regression because ./mach repackage msix did not guess parameters at all before bug 1742998.

Yeah, it's really a regression, but certainly a bug in that work. I'll try to reproduce myself, but I would also be interested in seeing the mozconfig you're using.

Either way, this guessing needs to be more cautious about what's available -- clearly bindir is not guaranteed.

Comment 5

[Mitchell Hentges [:mhentges] 🦀]

I don't think command_context is a dictionary or otherwise has a get() function - do you want command_context._mach_context?
The naming here is awful, but the idea is:

• command_context: what self used to be in mach commands, carries topsrcdir, metrics, virtualenv_manager, etc
• mach_context carries other, global-to-Mach information, including perhaps bindir.

Comment 6

[[e-mail] (:bhearsum)]

(In reply to Mitchell Hentges [:mhentges] 🦀 from comment #5)

I don't think command_context is a dictionary or otherwise has a get() function - do you want command_context._mach_context?
The naming here is awful, but the idea is:

• command_context: what self used to be in mach commands, carries topsrcdir, metrics, virtualenv_manager, etc
• mach_context carries other, global-to-Mach information, including perhaps bindir.

Oh, yikes, how embarassing -- you're right. command_context has no get method. It does however, appear to hold bindir - at least in my non-artifact build:

(Pdb) dir(command_context)
['__class__', '__delattr__', '__dict__', '__dir__', '__doc__', '__eq__', '__format__', '__ge__', '__getattribute__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__le__', '__lt__', '__module__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__', '__weakref__', '_config_environment', '_ensure_objdir_exists', '_ensure_state_subdir_exists', '_ensure_zstd', '_get_state_filename', '_is_osx', '_is_windows', '_logger', '_mach_context', '_make', '_mozconfig', '_normalize_command', '_run_command_in_objdir', '_run_command_in_srcdir', '_run_make', '_set_log_level', '_spawn', '_sub_mach', '_topobjdir', '_virtualenv_manager', '_virtualenv_name', '_wrap_path_argument', 'activate_virtualenv', 'backend_out_of_date', 'bindir', 'build_out_of_date', 'config_environment', 'defines', 'distdir', 'from_environment', 'get_binary_path', 'get_mozconfig_and_target', 'includedir', 'is_clobber_needed', 'log', 'log_manager', 'metrics', 'mozbuild_reader', 'mozconfig', 'mozconfig_and_target', 'notify', 'platform', 'populate_logger', 'python3', 'reload_config_environment', 'repository', 'resolve_config_guess', 'resolve_mozconfig_topobjdir', 'run_process', 'settings', 'statedir', 'substs', 'topobjdir', 'topsrcdir', 'virtualenv_manager']
(Pdb) dir(command_context._mach_context)
['__class__', '__delattr__', '__dict__', '__dir__', '__doc__', '__eq__', '__format__', '__ge__', '__getattribute__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__le__', '__lt__', '__module__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__', '__weakref__', 'command_attrs', 'commands', 'cwd', 'global_parser', 'handler', 'is_interactive', 'log_manager', 'pre_dispatch_handler', 'settings', 'telemetry', 'topdir']

Comment 7

[Mitchell Hentges [:mhentges] 🦀]

It does however, appear to hold bindir - at least in my non-artifact build:

Ah, you're right :)
A helpful link will be that command_context is an instance of MachCommandBase - there's bindir defined down here.

Comment 8

[[e-mail] (:bhearsum)]

Attachment: Bug 1743233: ./mach repackage fails to infer parameters. r?mhentges!

Comment 9

[Masatoshi Kimura [:emk]]

ac_add_options --target=x86_64-pc-mingw32
ac_add_options --host=x86_64-pc-mingw32
ac_add_options --with-ccache=sccache
mk_add_options AUTOCLOBBER=1
WIN32_REDIST_DIR="D:/Program Files (x86)/Microsoft Visual Studio/2017/Community/VC/Redist/MSVC/14.15.26706/x64/Microsoft.VC141.CRT"
WIN_UCRT_REDIST_DIR="D:/Program Files (x86)/Windows Kits/10/Redist/ucrt/DLLs/x64"

This is my mozconfig.

Comment 10

[[e-mail] (:bhearsum)]

(In reply to Masatoshi Kimura [:emk] from comment #9)

ac_add_options --target=x86_64-pc-mingw32
ac_add_options --host=x86_64-pc-mingw32
ac_add_options --with-ccache=sccache
mk_add_options AUTOCLOBBER=1
WIN32_REDIST_DIR="D:/Program Files (x86)/Microsoft Visual Studio/2017/Community/VC/Redist/MSVC/14.15.26706/x64/Microsoft.VC141.CRT"
WIN_UCRT_REDIST_DIR="D:/Program Files (x86)/Windows Kits/10/Redist/ucrt/DLLs/x64"

This is my mozconfig.

Thank you! (In reply to Masatoshi Kimura [:emk] from comment #9)

ac_add_options --target=x86_64-pc-mingw32
ac_add_options --host=x86_64-pc-mingw32
ac_add_options --with-ccache=sccache
mk_add_options AUTOCLOBBER=1
WIN32_REDIST_DIR="D:/Program Files (x86)/Microsoft Visual Studio/2017/Community/VC/Redist/MSVC/14.15.26706/x64/Microsoft.VC141.CRT"
WIN_UCRT_REDIST_DIR="D:/Program Files (x86)/Windows Kits/10/Redist/ucrt/DLLs/x64"

This is my mozconfig.

Thank you! I tested my patch here with your mozconfig and this should be fixed once it lands.

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1742998

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cd88dd2d5b93
/mach repackage fails to infer parameters. r=mhentges

Comment 13

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/cd88dd2d5b93

Comment 14

[Masatoshi Kimura [:emk]]

Verified fixed. Thanks for the quick fix! Now my experience has improved as bug 1742998 intended.