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

building: makespec: add new command-line switches for hookutils functions #5391

Merged
merged 2 commits into from Apr 10, 2021

Conversation

rokm
Copy link
Member

@rokm rokm commented Dec 16, 2020

Add new command-line options that map to the corresponding functions from the hookutils:

  • --collect-submodules: collect_submodules()
  • --collect-data: collect_data_files()
  • --collect-binaries: collect_dynamic_libs()
  • --collect-all: collect_all()
  • --copy-metadata: copy_metadata()

The hookutils calls are added to the generated .spec files. They may be specified multiple times, and their results supplement
the entries that are manually passed via existing --add-data, --add-binary, and --hidden-import switches.

This allows hookutils calls to be added from command-line, without having to manually edit the .spec file.

@rokm rokm force-pushed the command-line-collection branch 2 times, most recently from 847a5df to f006b8c Compare December 16, 2020 12:16
@bwoodsend
Copy link
Member

Ah, brilliant. I've been meaning to do this for ages.

@bwoodsend
Copy link
Member

--collect-submodules=foo is already implemented as --hiddenimport=foo.* but I think --collect-submodules is more intuitive.

…ions

Add new command-line options that map to the corresponding functions
from the hookutils:
* --collect-submodules: collect_submodules()
* --collect-data: collect_data_files()
* --collect-binaries: collect_dynamic_libs()
* --collect-all: collect_all()
* --copy-metadata: copy_metadata()

The hookutils calls are added to the generated .spec files. They
may be specified multiple times, and their results supplement
the entries that are manually passed via existing --add-data,
--add-binary, and --hidden-import switches.

This allows hookutils calls to be added from command-line, without
having to manually edit the .spec file.
@rokm rokm marked this pull request as ready for review December 28, 2020 11:31
@htgoebel
Copy link
Member

I'm not convinced we should add more command-line options. Look at the many options available in the .spec-file. If you try to support all these, this will create a command-line interface with many, many options. And this is terrible UX.

@htgoebel htgoebel added DO NOT MERGE kind:to be discussed Please add our opionon and rational labels Jan 11, 2021
@bwoodsend
Copy link
Member

this will create a command-line interface with many, many options. And this is terrible UX.

What makes you say that lot's of options is bad user experience?

Personally, I find:

$ pyinstaller -h
pages and pages of stuff
half of which I've never ever used
some of which is for a different operating system

to be a pain. I almost always need grep to find what I'm looking for (normally something along the lines of is it --hiddenimport, --hidden or --hidden-import?) I'd prefer the output of help to be abbreviated by default, with an option to give more details on a specific topic. And maybe group the options like gcc does, use --help=topic to get detailed descriptions within a group?

I've had to walk inexperienced users through switching to the spec file in order to use one of these hook utilities far too many times whereas I've never used an option like --win-no-prefer-redirects so if it's a matter of limiting the number of CLI options, I'd start jettisoning some of the less useful ones to make space for these new hook utility ones.

@bwoodsend bwoodsend merged commit 5898725 into pyinstaller:develop Apr 10, 2021
@rokm rokm deleted the command-line-collection branch May 5, 2021 09:48
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
DO NOT MERGE kind:to be discussed Please add our opionon and rational
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants