Use sdist from setuptools not distutils #66
Conversation
|
Thanks for the quick release! |
|
Isn't this a direct revert of @jasongrout 's previous change? 8853c5a / #51 |
|
Indeed it looks like it - if both use cases are valid maybe the behaviour could be toggled with a kwarg? |
|
It seems the better thing to do is to fix jlab by including the package data files in it's manifest.in. I think the problem in jlab is essentially what is documented here: pypa/setuptools#1461 |
In jupyter/jupyter-packaging#66, jupyter_packaging switched (back) to using setuptools rather than distutils for sdist. We had problems before with the staging directory not being included when using the setuptools sdist (see conda-forge/jupyterlab-feedstock#235 and jupyter/jupyter-packaging#51). It seems that the real problem we were experiencing was that when you have include_package_data=True, setuptools apparently does not include package_data files that are not given in MANIFEST.in (see pypa/setuptools#1461, for example). This tries to get our packaging working with setuptools. We’ll be able to test this better when we actually do a release. I tested with just running `python setup.py sdist` in a fresh checkout, and it ended up building the javascript in the staging directory, and then it included the build and node_modules directory. Thus I prune these out explicitly here. However, I think our normal build process does not build in that directory, but rather copies over files from the dev_mode directory, so the prune may not be needed in our normal build process. It doesn’t hurt to have it in, though. It may be that we can consolidate these include statements, for example, perhaps we can just do “graft jupyterlab/staging”. However, I went for simplicity in closely mirroring what we have in package_data in setup.py.
I submitted jupyterlab/jupyterlab#9780 to try to do this. |
|
Right, but even if we fix it for lab, that project isn't the only user of this package... So to be clear, is lab doing something special that makes this unlikely to break for someone else as well, or do all projects relying on jupyter_packaging need to make a change now? |
|
For clarity: I'm not saying I oppose the move to setuptools, but I am pointing out that this is a breaking change (I verified it breaking in another project by installing manifix and running |
|
Turns out my verification was thrown off by some other spurious files, it might not be as breaking as I thought... (yay 🎉 ) |
Currently, if using jupyter-packaging in a package with
pyproject.tomland building an sdist withpython -m build --sdist, many package files, includingsetup.pyas well as files defined inMANIFEST.indon't get included in the source distribution. This is solved by wrapping setuptools' rather than distutils'sdistcommand.