Switch setup.py to use setuptools #68
Comments
|
Thanks for bringing this up. I've been fighting distutils lately, so I might give this switch a try. |
|
@dvander One suggestion I want to make for the switch (if you go for it) is to drop the 'scripts' folder. Setuptools includes a handy 'entry_points' keyword argument in the 'setup' function, and a special 'console_scripts' entry. for AMBuild2, I recommend an entry like: 'ambuild = ambuild2.run:cli_run'. Setuptools then generates a platform appropriate script (executable on Windows) that'll invoke the entry point. |
|
Infact @dvander , while quickly checking the setup.py script and setuptools' documentation, theoretically you can just straight up swap the target import from distutils to setuptools and be done with it. |
|
That's good to know. AMBuild is due for some maintenance (I have a large TODO to fix how to detects MSVC) so I can give this a try soon. |
|
@dvander If you would allow me, I can quickly get a Pull Request up to pull the trigger on the switch. |
Fixes alliedmodders#68: See [bpo-41282](https://bugs.python.org/issue41282) for switch reason.
Fixes alliedmodders#68: See [bpo-41282](https://bugs.python.org/issue41282) for switch reason.
Something doesn't work here on Ubuntu 20 for me. The generated "ambuild" script fails with the above error. When I restore the old ambuild script, it works fine. |
|
That error reads that it was unable to load the entry point. The entry point is 'cli_run' in run.py, does it exist? |
|
I was unable to replicate your issue with Ubuntu 20.04 (via Windows Subsystem for Linux): I was only able to replicate after running the following command: |
|
@dvander Managed to actually replicate the issue you have: The cause: Your prior installation of AMBuild |
|
Clarification: This will ALWAYS fail for disutils installed packages. |
|
How do we fix this? |
|
Unfortunately, it's a limitation of pip and setuptools, they explicitly complain that disutils installed packages can't be reliabily uninstalled. Mitigation wise is warning people about having preinstalled previous versions. |
|
It seems like the setuptools script should error if there is a distutils version installed. I wound up with a broken installation which is not very good. |
|
setuptools doesn't error out unfortunately, we could raise it as a issue
for setuptools.
pip on the other hand WILL error out if it detects a distutils installed
package.
…On Thu., Jul. 30, 2020, 2:39 p.m. David Anderson, ***@***.***> wrote:
It seems like the setuptools script should error if there is a distutils
version installed. I wound up with a broken installation which is not very
good.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#68 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABGFQEZJIM5IYZTI77OJ6YLR6G443ANCNFSM4O2WIXZQ>
.
|
|
There's no way to have the new installer check that a version was installed with the previous installer? Why are we using this new installer, then? :) |
|
It was most likely done this was as the Python community has accepted 'pip'
as the package manager.
'pip list'
setuptools/distutils are just frameworks for building packages/installing
source distributions.
…On Thu., Jul. 30, 2020, 2:48 p.m. David Anderson, ***@***.***> wrote:
There's no way to have the new installer check that a version was
installed with the previous installer? Why are we using this new installer,
then? :)
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#68 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABGFQEY733CMCHXP47DD6VLR6G6AFANCNFSM4O2WIXZQ>
.
|
|
If there is literally no way to have the new installer check and error on a distutils install - I'd like to revert this change. A warning is good in principle, but will get lost in the gratuitous installation spew - it has to be an error. It seems hard to believe setuptools is missing a basic principle of package management - there's no way to run an arbitrary script as a prerequisite, where we could check if there is a distutils version of the same package? |
|
Like I said, pip is the package manager in the Python community.
I also wager that no matter the tool we use, NONE OF THEM has package management capabilities.
Primarily, they're used to build distributions (wheels are the goto preferred choice) and tools (namely pip) takes these distributions and installs them.
Source Distributions require the framework to be installed to... well... install the package.
…On Thu., Jul. 30, 2020, 3:10 p.m. David Anderson, ***@***.***> wrote:
If there is literally *no way* to have the new installer check and error
on a distutils install - I'd like to revert this change. A warning is good
in principle, but will get lost in the gratuitous installation spew - it
has to be an error.
It seems hard to believe setuptools is missing a basic principle of
package management - there's no way to run an arbitrary script as a
prerequisite, where we could check if there is a distutils version of the
same package?
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#68 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABGFQE3ZBQ5LRNXM3WWL3TLR6HAKJANCNFSM4O2WIXZQ>
.
|
|
I'd be fine moving to pip, but no matter what we do, we need some breaker to make sure we don't create two conflicting installs. It's just a terrible experience - I lost an hour trying to figure out what was going on, and I'm sure someone less involved than me would give up and conclude AMBuild was broken. Looking at setup.py, it's just a normal Python script. It seems like we should be able to throw some detection logic in there and prevent it from running. |
|
I can look tonight to see if there's anything in the distutils API for doing this. |
|
@dvander Can we move this discussion to a new issue, incase we do get
anyone else with this problem?
…On Thu., Jul. 30, 2020, 3:47 p.m. David Anderson, ***@***.***> wrote:
I can look tonight to see if there's anything in the distutils API for
doing this.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#68 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABGFQE4UE4U73OO46DSDCJDR6HE43ANCNFSM4O2WIXZQ>
.
|
|
@dvander There may not even be API for that other than potentially utilizing pip's code, the easiest solution would be adding an easy to access advisory warning and heavily request people install AMBuild using 'pip' example pip command: 'pip install <path_to_ambuild_dir>' Another thing to note, should distutils become depreciated and removed, only setuptools will provide a copy of it, and even then, they'll most likely redirect ALL utilizations to setuptools itself. Pip already does this when it goes and installs distutils packages. |
|
@dvander I think I may have found a more reliable "dummy solution": distutils creates a file with the extension ".egg-info" and plops it alongside the ambuild packages, wheel/egg installs on the otherhand keep this file in a sub-folder instead. |
|
If in the objdir/sourcedir, that sounds unideal since then a second copy would break. |
|
I was talking about the site_packages folder. Anyway, incase you didn't look at the setuptools issue I created, there is plans to depreciate the use of 'setup.py install' and that using that can break Python environments... |
|
@dvander Is it alright if I file pull requests with all projects that install AMBuild if it's not already installed, these pull requests will swap the checkout-deps scripts to using |
|
Yup, that's fine, we should change the AMBuild documentation as well. |
|
And, if "setup.py install" shouldn't be used, we should forbid using it. |
|
Ok, I'll begin setting up PRs for the repos who's checkout-deps scripts install AMBuild. |
|
@dvander PRs are done, AMTL gave me the worst headache to deal with though... |
|
Only SourceMod's remains (It also has a SM issue tied to it) |
|
I think this can now officially be closed. Any further issues regarding this change should go into it's own issue now. |
|
Remaining: need to block CLI setup.py installs and change documentation. I can do the latter pretty quickly, not sure about the former. |
|
The former has to be done on the pip/setuptools end, the only other thing we could do is switch to PEP 517 format, but that is experimental at best. |
|
@dvander I also made some documentation changes for AMBuild, I also fixed the 'pip install' command on it as what you wrote fails (as PIP would look on PyPi instead of locally) |
|
This remains open (and reverts possible) until we've addressed all the installation issues. I'll take a look tonight. |
|
On the Pull Request I opened for Steamworks, you mentioned that CI should be using tagged releases of AMBuild and not straight git cloning, I propose we switched to tagged releases and provide universal wheels that can be downloaded and installed. This will accomplish restricting the usage of Building wheels requires setuptools as distutils doesn't include 'bdist_wheel' command Edit: Yes, install source zips is supported by PIP. |
|
I don't particularly care how CIs install ambuild, as long as it works. They always have the option of pinning to a specific rev. I just created the "2.1-distutils" tag so SteamWorks can checkout to that version as a quick fix if desired. |
|
Actually, you needed to checkout the commit first before creating the tag. Here's the command: |
|
Is my attempt to fix this. I've tested it locally, and I get: It's by no means perfect but unless there's anything better, I'm happy to settle it at that. |
|
Thanks for updating the doc. Aside from third-party CI (which can check out an older rev), I think this is all in a much better state now. pip is much better than distutils. |
Discussion is currently going on at bpo-41282 to depreciate and remove distutils, AMBuild uses distutils for installation, but it doesn't support egg info generation which is required to uninstall AMBuild with pip.
Note: pip can install source distributions from an AMBuild repo clone/zip extract.
Note 2: setuptools mimics distutils to the point where minimal work will be required for AMBuild to switch.
The text was updated successfully, but these errors were encountered: