Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --no-sign as a no-op to Windows/Linux/Android packaging platforms #486

Closed
GenevieveBuckley opened this issue Sep 9, 2020 · 8 comments
Labels
enhancement New features, or improvements to existing features.

Comments

@GenevieveBuckley
Copy link
Contributor

Is your feature request related to a problem? Please describe.
It is annoying to have special handling for the syntax of briefcase package commands, depending on whether you are packaging for Mac or Windows/Linux.

As suggested by Russell in this comment

Adding --no-sign support for other platforms (even if it's no-op) would also simplify things nicely.

Describe the solution you'd like
We'd like to have briefcase package accept a --no-sign argument regardless of the OS platform it's running on, and just have it be a no-op (no operation, do nothing) for Windows/Linux/Android.

Describe alternatives you've considered
Right now I use if statements in my github actions to get around this.

Additional context
You can see the kind of special case handling we currently need to do to work around this in these lines of the traveltips github action to package and upload installers

@GenevieveBuckley GenevieveBuckley added the enhancement New features, or improvements to existing features. label Sep 9, 2020
@freakboy3742 freakboy3742 added enhancement New features, or improvements to existing features. good first issue Is this your first time contributing? This could be a good place to start! up-for-grabs and removed enhancement New features, or improvements to existing features. labels Sep 9, 2020
@ElijahAhianyo
Copy link
Contributor

hi, first timer here, can i take this up?

@freakboy3742 freakboy3742 removed good first issue Is this your first time contributing? This could be a good place to start! up-for-grabs labels Sep 11, 2020
@freakboy3742
Copy link
Member

@ElijahAhianyo Absolutely - go right ahead! If you need any pointers or suggestions, just ask - we're happy to provide any assistance we can.

@ElijahAhianyo
Copy link
Contributor

@freakboy3742 Great! I took a look at the src/briefcase/cmdline.py file and I figured adding an argument like so would do:

parser.add_argument(
        '--no-sign',
        metavar='',
        help="<fix help message here>"
    )

I'm not exactly sure if I'm on track or doing what's required. Do you have any suggestions on how this can be handled?

@freakboy3742
Copy link
Member

That's the right argument to add; but the wrong place. cmdline.py applies to all commands. As this is an argument that is specific to the Publish command, it only needs to be visible on the Publish command.

If you take a look at the macOSAppPackageCommand in platforms/macOS/app.py, you'll see the mechanism by which --no-sign is currently added. If that mechanism is migrated up the inheritance tree to the base Publish command, it will become a shared command across all Publish commands. You'll also need to tweak the help text so it's not macOS specific.

As an aside, I imagine it would be worth migrating the "adhoc" and "identity" command options up as well; those are likely to be needed on other platforms. The exact format needed for "identity" will probably vary between platforms, but we can deal with that when we make the argument not a no-op.

@ElijahAhianyo
Copy link
Contributor

right. im guessing i'd have to move the command options to the parent class(src/briefcase/commands/package.py).
Also, can you elaborate on what you mean by tweaking the help text? I guess what I'm trying to say is, based on my observation, the help text of the command options(ad-hoc,identity and no-sign) are not macOs specific unless otherwise

@freakboy3742
Copy link
Member

The code move you've suggested sounds correct.

As for the help text - the current text talks about ".app bundles", which is macOS terminology. The "generic" version should refer to "your app" (or whatever wording makes sense grammatically). The guidance about "40-digit hex or full name of identity", won't necessarily be accurate on other platforms either - but until we know what will be accurate, we might as well leave it as-is.

There's also the help text in the documentation. The docs currently list the signing options as a macOS specific extension on the app and dmg commands; this documentation should be moved to the reference docs for the package command.

@ElijahAhianyo
Copy link
Contributor

sure. ill do just that. Thanks a lot for taking the time to give detailed explanations and stating things clear. Means a lot to me. I'll go ahead and open a PR asap

@freakboy3742
Copy link
Member

Fixed by #491

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features, or improvements to existing features.
Projects
None yet
Development

No branches or pull requests

3 participants