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 bootstrap script/wheel module for flit_core #511

Merged
merged 6 commits into from
Feb 21, 2022

Conversation

jameshilliard
Copy link
Contributor

Improved version of #481 based on comments here.

Details from help:

$ python -m flit_core.wheel --help
usage: wheel.py [-h] [--srcdir SRCDIR] [--outdir OUTDIR]

options:
  -h, --help            show this help message and exit
  --srcdir SRCDIR, -s SRCDIR
                        source directory (defaults to current directory)
  --outdir OUTDIR, -o OUTDIR
                        output directory (defaults to /Users/jameshilliard/flit/flit_core/dist)
$ python bootstrap.py --help
usage: bootstrap.py [-h] {build,install} ...

positional arguments:
  {build,install}

options:
  -h, --help       show this help message and exit
$ python bootstrap.py build --help
usage: bootstrap.py build [-h] [--outdir OUTDIR]

options:
  -h, --help            show this help message and exit
  --outdir OUTDIR, -o OUTDIR
                        output directory (defaults to /Users/jameshilliard/flit/flit_core/dist)
$ python bootstrap.py install --help
usage: bootstrap.py install [-h] [--wheeldir WHEELDIR] [--installdir INSTALLDIR]

options:
  -h, --help            show this help message and exit
  --wheeldir WHEELDIR, -w WHEELDIR
                        wheel dist directory (defaults to /Users/jameshilliard/flit/flit_core/dist)
  --installdir INSTALLDIR, -i INSTALLDIR
                        installdir directory (defaults to /Users/jameshilliard/flit/venv/lib/python3.10/site-packages

flit_core/bootstrap.py Outdated Show resolved Hide resolved
@FFY00
Copy link
Member

FFY00 commented Jan 27, 2022

Ah, I think I missed something, shouldn't this be installed? If so, this should live under the flit_core module, and accessible via python -m flit_core.bootstrap.

I will look at this in more detail later.

@jameshilliard
Copy link
Contributor Author

Ah, I think I missed something, shouldn't this be installed?

Only the wheel builder is installed since the bootstrap.py installer is only designed to install flit_core itself, it's expected that installer be used to install wheels built using python -m flit_core.wheel. This should work for bootstrapping since installer only depends on flit_core.

@takluyver
Copy link
Member

Thanks for this. In the interests of keeping the extra script as simple as possible, and minimising coupling between the script and the internals of flit_core, I'd like to have the script only do installation, and use python -m flit_core.wheel directly to build a wheel. From what we've discussed this is the same command you'll use to build wheels for a few subsequent packages (e.g. installer, tomli), so I don't think it should be a problem to use it for flit_core itself as well.

I know what I want this to look like, so I hope you won't mind if I just make the changes directly rather than asking you to do them.

@takluyver
Copy link
Member

OK, I hope this works for you.

I've got rid of the globbing inside the script - if you know there's only one suitable wheel in a directory, you can always use dist/flit_core-*.whl in a script calling it. And I've got rid of the fallback to building a wheel, because I want this to do just one simple thing, not have different code paths. You've mentioned in the past that you separate build & install steps, so I assume you don't need one script combining them (and if anyone does, it should be easy to write that themselves).

@takluyver takluyver added this to the 3.7 milestone Feb 16, 2022
@jameshilliard
Copy link
Contributor Author

In the interests of keeping the extra script as simple as possible, and minimising coupling between the script and the internals of flit_core, I'd like to have the script only do installation, and use python -m flit_core.wheel directly to build a wheel.

Yeah, I don't think that should be an issue, I had just attempted to preserve the combo build+install option from #481 but for our use case where we do build+install stage separation a combo build+install is not actually needed.

I know what I want this to look like, so I hope you won't mind if I just make the changes directly rather than asking you to do them.

OK, I hope this works for you.

Yeah, I think this should work for us.

@takluyver
Copy link
Member

Great, thanks. I've updated the bootstrapping doc. I'll give it another day or two in case other maintainers want to look at it, but otherwise I think this can go in.

Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@takluyver takluyver merged commit ced1ef7 into pypa:main Feb 21, 2022
@jameshilliard jameshilliard deleted the bootstrap branch August 3, 2022 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants