Open Bug 1896170 Opened 2 months ago Updated 3 days ago

`CheckArg` doesn't support the `--flag=value` form

Categories

(Toolkit :: Startup and Profile System, defect)

Product:
Toolkit
Component:
Startup and Profile System
Version:
Firefox 125
Type:
defect

Tracking

()

Status:
ASSIGNED

People

(Reporter: christophe, Assigned: christophe)

Details

Attachments

(1 file, 6 obsolete files)

Bug 1896170 - add support for --option=param command line arguments. r=mossop
48 bytes, text/x-phabricator-request
Details | Review

Steps to reproduce:

I tried to launch a second instance of firefox with the command line on linux.
firefox ignores the --profile argument when the value is attached with =. It then tried to start the default profile.
Surprisingly firefox accept the argument --remote-debugging-port=9333.
I didn’t test the other command line arguments.

Actual results:

when the command line argument is "--profile=/...", firefox ignores the argument. When it is written "--profile /..." it is taken in account.

Expected results:

Firefox should accept the arguments with value in both forms "--arg=value" and "--arg value".

My version of firefox is 125.0.2 (64bits) in snap for Ubuntu of amd64.
This bug prevents me to use go-rod a golang module allowing to control a browser. It is equivalent to selenium or puppeteer. I don’t know if firefox can be used with them. It’s not advertised. I would like to automate tasks with firefox.

The Bugbug bot thinks this bug should belong to the 'Toolkit::Startup and Profile System' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → Startup and Profile System
Product: Firefox → Toolkit

Our command line argument handling is a bit messy and inconsistent unfortunately. A number of arguments that are handled in early startup are processed by CheckArg which doesn't support the --flag=value form. Most of the arguments that are handled later, after the core of Gecko has started up, do support it because they are processed by an entirely different function. This is pretty low priority to fix but I'd accept a patch to do so.

Severity: -- → S4
Summary: the command line parameter --profile doesn’t accept value attached with = → `CheckArg` doesn't support the `--flag=value` form

I’ll see what I can do. Thank you for taking to time to look into the problem and answering.

Attached patch cmdLine.patch (obsolete) — — Splinter Review

This patch adds support for options with the parameter attached with = as in --option=param. It is intended to preserve backward compatibility.

The function CheckArg in the file toolkit/xre/CmdLineAndEnvUtils.h has been rewritten to make it more readable.

I added two helper functions in the internal namespace:

  • isValidOptionName checks the validity of the option name to look lookup in the argument list
  • optionMatch checks if the given argument is the searched option

I’m currently unable to build firefox because there is a problem with the rust uniffi thing.
I thus couldn’t test that the patch is working. But it passes static compiler checking.

Would you be able to submit the patch for code review via phabricator? See the docs.

Flags: needinfo?(christophe)

The function CheckArg in the file toolkit/xre/CmdLineAndEnvUtils.h has been
rewritten to make it more readable.

Two helper functions have been added in the internal namespace:

  • isValidOptionName checks the validity of the option name to lookup in the
    argument list
  • optionMatch checks if the given argument is the searched option
Assignee: nobody → christophe
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

I'm struggling to understand whether all these patches are still necessary and if so which order they are meant to be applied in. It would be easier to include all the changes in a single patch

A single commit would probably do as well since the changes are minimal.
Unfortunately I’m not familiar with mercurial, moz-phab and phabricator.

May I start from scratch, combine all the changes in a new single commit and publish the patch ? Would that work ?
Will the previous patch be garbage collected or do I have to delete them explicitly ?

Flags: needinfo?(christophe)

I reinstalled firefox and managed to compile and run it. I should now be able to test my changes before committing and publishing them.

The function CheckArg in toolkit/xre/CmdLineAndEnvUtils.h has been rewritten
to make it more readable.

Three helper functions have been added in the internal namespace:

  • charimatch performs a case insensitive comparision of two chars
  • optionMatch checks if the given argument is the searched option
  • isValidOptionName checks the validity of the option name to lookup in the argument list

The function strimatch has been modified to use charimatch.

It’s fresh new patch.

Unfortunately, I could not test the code because I failed to build firefox without using the artifacts. Compiling firefox with the artifacts works but it doesn’t take in account the modified header file when rebuilding.

Building firefox without artifacts fails apparently due to a crash of the rustc compiler. It’s not an error in some code. It might be due to a compiler version incompatibility.

Please abandon the other revisions in phabricator so they are no longer up for review. hg has various ways to alter the existing commit, hg commit --amend or hg amend (if you have the evolve extension enabled), those are the preferred ways to alter a commit and then re-submit to phabricator.

Unfortunately I am out for the next week on vacation so I won't be able to look at this until I'm back. But in the meantime it would be good to add some tests to TestCmdLineAndEnvUtils.cpp.

The function CheckArg in toolkit/xre/CmdLineAndEnvUtils.h has been rewritten
to make it more readable.

Three helper functions have been added in the internal namespace:

charimatch performs a case insensitive comparision of two chars
optionMatch checks if the given argument is the searched option
isValidOptionName checks the validity of the option name to lookup in the argument list

The function strimatch has been modified to use charimatch.

Christophe, it's great that you are interested in fixing this in consistency! So thanks a lot for working on these patches. For now I wanted to ask about the state for reviews. At it looks like some revisions are in changes planned or needs revision state. Are you going to update these or are you waiting for the review of the other revisions first? Thanks!

Flags: needinfo?(christophe)

Hello, I'm quite busy these days. I'll be able to work on it next week only.

I'm facing three difficulties to work on this project.

  1. I can't compile firefox locally (Ubuntu 24.04). I thus can't compile and test the code before publishing
  2. I don't know how to run the tests locally, assuming step 1 was resolved
  3. I don't fully master the source code management system used by bugzilla

A little help would be welcome.

I republished the proposed code to resolve comments and linting errors, but I don't know if this is the way to do it.

Working on this bug is thus quite difficult for me in the current conditions. It would be OK with me if someone wanted to take over the code change.

Flags: needinfo?(christophe)

(In reply to Christophe Meessen from comment #19)

Hello, I'm quite busy these days. I'll be able to work on it next week only.

Thanks a lot for the time you spent already on making argument handling better. Let me see if I can help with the process...

  1. I can't compile firefox locally (Ubuntu 24.04). I thus can't compile and test the code before publishing

What is the exact error you get? I assume you run ./mach bootstrap and selected the full build of Firefox?

  1. I don't know how to run the tests locally, assuming step 1 was resolved

In case you don't know which exact harness runs the tests you could easily use ./mach test %path_to_file_or_folder% to run the tests at this location. I would suggest to do that first once the build step is working.

  1. I don't fully master the source code management system used by bugzilla

In case you have specific questions please let us know and we can help you with the remaining issues.

A little help would be welcome.

Sure, whatever we can do! I'm not a peer for this component but I can try my best given that I also find this annoying and would like to see this fixed.

Also for questions around the build process you could also check our #build channel on Matrix.

I can take over finishing this patch next week if it isn't otherwise moved forward by then.

Attachment #9401742 - Attachment is obsolete: true
Attachment #9403480 - Attachment is obsolete: true
Attachment #9401700 - Attachment is obsolete: true
Attachment #9403481 - Attachment is obsolete: true
Attachment #9403482 - Attachment is obsolete: true
Attachment #9403751 - Attachment is obsolete: true

(In reply to Dave Townsend [:mossop] from comment #21)

I can take over finishing this patch next week if it isn't otherwise moved forward by then.

That would be really great as I really have a hard time working on this patch.

I’ll try to help as much as I can. I’ll first try to compile firefox. Maybe Hendrik Skupin can help me.

(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #20)

(In reply to Christophe Meessen from comment #19)

Hello, I'm quite busy these days. I'll be able to work on it next week only.

Thanks a lot for the time you spent already on making argument handling better. Let me see if I can help with the process...

  1. I can't compile firefox locally (Ubuntu 24.04). I thus can't compile and test the code before publishing

What is the exact error you get? I assume you run ./mach bootstrap and selected the full build of Firefox?

Trying to reinstall from scratch, I’m afraid I have corrupted my mozilla build setup.
I erased .mozbuild, assuming that it would be recreated with a bootstrap and it isn’t.
Whenever I do a hg version, even in my home directory, I get the errors

$ hg version
*** failed to import extension "evolve" from /home/meessen/.mozbuild/evolve/hgext3rd/evolve: [Errno 2] No such file or directory: '/home/meessen/.mozbuild/evolve/hgext3rd/evolve'
*** failed to import extension "firefoxtree" from /home/meessen/.mozbuild/version-control-tools/hgext/firefoxtree: [Errno 2] No such file or directory: '/home/meessen/.mozbuild/version-control-tools/hgext/firefoxtree'
*** failed to import extension "clang-format" from /home/meessen/.mozbuild/version-control-tools/hgext/clang-format: [Errno 2] No such file or directory: '/home/meessen/.mozbuild/version-control-tools/hgext/clang-format'
*** failed to import extension "js-format" from /home/meessen/.mozbuild/version-control-tools/hgext/js-format: [Errno 2] No such file or directory: '/home/meessen/.mozbuild/version-control-tools/hgext/js-format'
*** failed to import extension "push-to-try" from /home/meessen/.mozbuild/version-control-tools/hgext/push-to-try: [Errno 2] No such file or directory: '/home/meessen/.mozbuild/version-control-tools/hgext/push-to-try'
Mercurial Distributed SCM (version 6.7.2)
(see https://mercurial-scm.org for more information)

Copyright (C) 2005-2023 Olivia Mackall and others
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

How can I fix these errors ? The ./mach command didn’t have a "clean" command. I’m really confused by the firefox development environment.

So some of the hg configuration you had to make ends-up in your global user settings for Mercurial. Please have a look into that config file that you can find as ~/.hgrc, and check if any extension (like the ones as listed above) reference the no longer existing .mozbuild folder. Cleaning that up should help you to get mach working again.

Too many errors when trying to compile. It’s hopeless.

0:18.34    Compiling rand_chacha v0.3.1
 0:19.07 error: rustc interrupted by SIGSEGV, printing backtrace
 0:19.08 /home/meessen/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/../lib/librustc_driver-d6f66a8619a171d6.so(+0x2e40fd3)[0x701180240fd3]
 0:19.08 /lib/x86_64-linux-gnu/libc.so.6(+0x45320)[0x70117d045320]
 0:19.08 /home/meessen/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/../lib/../lib/libLLVM.so.18.1-rust-1.78.0-stable(_ZN4llvm23removeUnreachableBlocksERNS_8FunctionEPNS_14DomTreeUpdaterEPNS_16MemorySSAUpdaterE+0x39f)[0x70117b4a079f]
 0:19.08 /home/meessen/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/../lib/../lib/libLLVM.so.18.1-rust-1.78.0-stable(_ZN4llvm13GlobalOptPass3runERNS_6ModuleERNS_15AnalysisManagerIS1_JEEE+0xfd1)[0x70117b49f191]
 0:19.08 /home/meessen/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/../lib/../lib/libLLVM.so.18.1-rust-1.78.0-stable(+0x5e9e1a3)[0x70117b49e1a3]
 0:19.08 /home/meessen/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/../lib/../lib/libLLVM.so.18.1-rust-1.78.0-stable(_ZN4llvm11PassManagerINS_6ModuleENS_15AnalysisManagerIS1_JEEEJEE3runERS1_RS3_+0xe1)[0x70117b84fd31]
 0:19.08 /home/meessen/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/../lib/librustc_driver-d6f66a8619a171d6.so(LLVMRustOptimize+0x822)[0x70118206c76e]
 0:19.09 /home/meessen/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/../lib/librustc_driver-d6f66a8619a171d6.so(+0x4c6a716)[0x70118206a716]
 0:19.09 /home/meessen/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/../lib/librustc_driver-d6f66a8619a171d6.so(+0x4c6a286)[0x70118206a286]
 0:19.09 /home/meessen/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/../lib/librustc_driver-d6f66a8619a171d6.so(_RNvXs1_CskFMTEvEfYcy_18rustc_codegen_llvmNtB5_18LlvmCodegenBackendNtNtNtCskzQGxkPzHAl_17rustc_codegen_ssa6traits5write19WriteBackendMethods13optimize_thin+0x60e)[0x701181fc0d24]
 0:19.09 /home/meessen/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/../lib/librustc_driver-d6f66a8619a171d6.so(+0x4bc6a3c)[0x701181fc6a3c]
 0:19.10 /home/meessen/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/../lib/librustc_driver-d6f66a8619a171d6.so(+0x4bc60a3)[0x701181fc60a3]
 0:19.10 /home/meessen/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/../lib/libstd-d2ef02247056996e.so(rust_metadata_std_e4dfbc2c3f4b09f1+0xc1145)[0x70118354d145]
 0:19.10 /lib/x86_64-linux-gnu/libc.so.6(+0x9ca94)[0x70117d09ca94]
 0:19.10 /lib/x86_64-linux-gnu/libc.so.6(+0x129c3c)[0x70117d129c3c]
 0:19.10 note: we would appreciate a report at https://github.com/rust-lang/rust
 0:19.44    Compiling generic-array v0.14.6
 0:19.84    Compiling time v0.1.45
 0:20.30 error: could not compile `syn` (lib)
 0:20.31 Caused by:
 0:20.34   process didn't exit successfully: `/home/meessen/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/rustc --crate-name syn --edition=2021 /home/meessen/Sources/firefox/mozilla-unified/third_party/rust/syn/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C opt-level=1 -C embed-bitcode=no --cfg 'feature="clone-impls"' --cfg 'feature="default"' --cfg 'feature="derive"' --cfg 'feature="extra-traits"' --cfg 'feature="fold"' --cfg 'feature="full"' --cfg 'feature="parsing"' --cfg 'feature="printing"' --cfg 'feature="proc-macro"' --cfg 'feature="quote"' --cfg 'feature="visit"' --cfg 'feature="visit-mut"' -C metadata=27ac82147d0dead8 -C extra-filename=-27ac82147d0dead8 --out-dir /home/meessen/Sources/firefox/mozilla-unified/obj-x86_64-pc-linux-gnu/release/deps -C linker=/home/meessen/Sources/firefox/mozilla-unified/build/cargo-linker -L dependency=/home/meessen/Sources/firefox/mozilla-unified/obj-x86_64-pc-linux-gnu/release/deps --extern proc_macro2=/home/meessen/Sources/firefox/mozilla-unified/obj-x86_64-pc-linux-gnu/release/deps/libproc_macro2-44af3363d830b19c.rmeta --extern quote=/home/meessen/Sources/firefox/mozilla-unified/obj-x86_64-pc-linux-gnu/release/deps/libquote-ec682f83838d0aaf.rmeta --extern unicode_ident=/home/meessen/Sources/firefox/mozilla-unified/obj-x86_64-pc-linux-gnu/release/deps/libunicode_ident-53204689596409ea.rmeta --cap-lints allow` (signal: 11, SIGSEGV: invalid memory reference)

I’m sorry, but I can’t contribute in these conditions. There are too many errors. It even froze my computer twice.

The errors are apparently mainly due to the rust compiler that keeps doing segmentation faults. The version of my rust compiler is

rustc 1.78.0 (9b00956e5 2024-04-29)

Thanks for the details! The feedback that I got from an engineer is that this really sounds like a hardware failure on your machine. Especially the froze my computer twice. So maybe Dave should indeed help out to finalize this patch.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: