Add the ability to override app configurations at the command line #1542

freakboy3742 · 2023-11-16T06:55:39Z

Adds a -C / --config option that allows a user to override the value of a setting at the command line.

e.g.:

briefcase create -C template=\"https:/example.com/template\"

would generate an app using the values in the pyproject.toml, except for using a different template.

This is primarily useful for internal testing setups, such as generating a test app inside a template repository, or running CI for the Apple support package in CI. However, it could also be useful to pass in other values, such as a version number.

It's likely only of any practical use in the create command, but it's been implemented in all "app facing" commands in case it might be useful.

Fixes #1115.

/cc @rmartin16

PR Checklist:

All new features have been tested
All new features have been documented
I have read the CONTRIBUTING.md file
I will abide by the code of conduct

rmartin16

Well timed; this is meeting my needs of creating specific projects in CI.

briefcase create linux AppImage \
    --config 'template="../../"' \
    --config 'requires=["toga-gtk==0.3.1", "PyGobject<3.46.0"]'

I guess I hadn't considered this before but...apparently attributes can have spaces.

briefcase package -C '"zzz template"={"asdf"=42}'

I gave up trying to get a newline character in to the value for -C....but that shouldn't matter since you can pass multiple values for -C.

👍🏼

rmartin16 · 2023-11-16T15:25:05Z

src/briefcase/commands/base.py

+            except ValueError:
+                raise BriefcaseCommandError(
+                    f"Unable to parse configuration override {value}"
+                )


I think Rich's stacktrace handles this regardless but can you raise from the ValueError?

Suggested change

except ValueError:

raise BriefcaseCommandError(

f"Unable to parse configuration override {value}"

)

except ValueError as e:

raise BriefcaseCommandError(

f"Unable to parse configuration override {value}"

) from e

That should be unnecessary: exceptions raised within an except block are automatically chained unless you say from None.

Might as well be explicit.

rmartin16 · 2023-11-16T15:25:31Z

src/briefcase/commands/base.py

+            dest="config_overrides",
+            action="append",
+            metavar="KEY=VALUE",
+            help="Override the value of the app configuration item KEY with VALUE.",


nitpick: no period at the end to match the other options (although, I also shouldn't have added one to -v...)

Suggested change

help="Override the value of the app configuration item KEY with VALUE.",

help="Override the value of the app configuration item KEY with VALUE",

Interesting - the very next option defined (--verbosity) did have a period. I've corrected both.

rmartin16 · 2023-11-16T15:29:21Z

src/briefcase/commands/run.py

@@ -248,6 +248,7 @@ def __call__(
        no_update: bool = False,
        test_mode: bool = False,
        passthrough: list[str] | None = None,
+        config_overrides: list[str] | None = None,


needs to be passed to finalize()?

Yes, but not any more with the revised approach that I've implemented.

rmartin16 · 2023-11-16T15:32:36Z

src/briefcase/commands/update.py

@@ -68,6 +68,7 @@ def __call__(
        update_resources: bool = False,
        update_support: bool = False,
        test_mode: bool = False,
+        config_overrides: list[str] | None = None,


needs to be passed to finalize()?

Again, Yes, but not any more with the revised approach that I've implemented.

mhsmith · 2023-11-16T20:33:15Z

@rmartin16: I'm guessing from your comments that you didn't intend to approve this yet?

mhsmith · 2023-11-16T20:48:52Z

src/briefcase/commands/base.py

+        for value in config_overrides:
+            try:
+                overrides.update(tomllib.loads(value))


I can't see any mention of multi-level config values, like --config 'linux.requires=...', which is valid TOML.

I think this code might actually work with that as long as you only had one of them. But then if you added --config linux.something_else=..., the second linux would overwrite the first instead of merging with it.

We don't necessarily need to support that syntax in this PR, but if we don't support it, we should at least display a useful error when the user tries to use it.

Good call. Right now, it's probably easier to reject multi-level keys completely. We can always add them later if there's a reasonable use case.

Actually - in the process of looking at this, it occurred to me that this approach avoids validation of any provided value (e.g., you could specify an invalid version number)... and when I went looking to see how I could do validation, I found a much cleaner way to handle the overrides, in a way that both validates and avoids the issue @rmartin16 found with not passing the overrides to some commands.

rmartin16 · 2023-11-16T20:58:32Z

I meant to....but selecting "comment" instead probably would have been a better choice perhaps. I was mostly trying to communicate "don't feel required to send this back to me with changes".

rmartin16

updating status to comment

freakboy3742 · 2023-11-17T04:47:58Z

I've updated the PR so that any command-line overrides go through the same validation process as if they were specified in pyproject.toml. This ends up being a lot less invasive on commands, in addition to the validation benefit.

I've also beefed up the validation that is done on the keys, ensuring that deep keys are rejected.

mhsmith · 2023-11-17T09:56:41Z

@rmartin16: FYI, if you want to cancel an approval, you have to choose "Dismiss review" under "Changes approved" at the bottom of the page.

freakboy3742 added 5 commits November 16, 2023 14:47

Add the ability to parse configuration overrides when finalizing apps.

5e62d3c

Pass config overrides through all app-facing commands.

806886e

Document the -C option.

115bae2

Add a changenote.

ee2e2c3

Fixes for Py3.8 compatibility.

8816ee0

freakboy3742 requested a review from mhsmith November 16, 2023 07:23

rmartin16 approved these changes Nov 16, 2023

View reviewed changes

mhsmith reviewed Nov 16, 2023

View reviewed changes

rmartin16 mentioned this pull request Nov 16, 2023

Add support for third-party GUI framework plugins #1524

Merged

4 tasks

rmartin16 reviewed Nov 16, 2023

View reviewed changes

rmartin16 mentioned this pull request Nov 17, 2023

Add reusable actions to create and/or verify a new project beeware/.github#68

Merged

4 tasks

freakboy3742 added 3 commits November 17, 2023 10:00

Remove periods from help text for consistency.

d37db2b

Explicitly pass exception context.

5dfd972

Rework override process so overrides are validated.

ead16a7

freakboy3742 requested a review from mhsmith November 17, 2023 04:43

Ensure that app_name isn't overridden, as it's an identifier.

7d102de

mhsmith approved these changes Nov 17, 2023

View reviewed changes

freakboy3742 merged commit fa275e8 into beeware:main Nov 17, 2023
34 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the ability to override app configurations at the command line #1542

Add the ability to override app configurations at the command line #1542

freakboy3742 commented Nov 16, 2023

rmartin16 left a comment

rmartin16 Nov 16, 2023

mhsmith Nov 16, 2023

freakboy3742 Nov 17, 2023

rmartin16 Nov 16, 2023

freakboy3742 Nov 17, 2023

rmartin16 Nov 16, 2023

freakboy3742 Nov 17, 2023

rmartin16 Nov 16, 2023

freakboy3742 Nov 17, 2023

mhsmith commented Nov 16, 2023

mhsmith Nov 16, 2023

freakboy3742 Nov 17, 2023

freakboy3742 Nov 17, 2023 •

edited

Loading

rmartin16 commented Nov 16, 2023

rmartin16 left a comment

freakboy3742 commented Nov 17, 2023

mhsmith commented Nov 17, 2023

	help="Override the value of the app configuration item KEY with VALUE.",
	help="Override the value of the app configuration item KEY with VALUE",

Add the ability to override app configurations at the command line #1542

Add the ability to override app configurations at the command line #1542

Conversation

freakboy3742 commented Nov 16, 2023

PR Checklist:

rmartin16 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhsmith commented Nov 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freakboy3742 Nov 17, 2023 • edited Loading

Choose a reason for hiding this comment

rmartin16 commented Nov 16, 2023

rmartin16 left a comment

Choose a reason for hiding this comment

freakboy3742 commented Nov 17, 2023

mhsmith commented Nov 17, 2023

freakboy3742 Nov 17, 2023 •

edited

Loading