pywin32 icon indicating copy to clipboard operation
pywin32 copied to clipboard

Add test for postinstall

Open Avasam opened this issue 1 year ago • 3 comments

I added it as an actual test in test files, rather than a CI only thing. Not sure if it belongs exactly there. Please let me know.

One thing I don't like is that it'll leave the dev's installation in a different state than they might've started it, maybe I should run

    find_and_run(maybes, ["-install"])
    find_and_run(maybes, ["-remove"])
    find_and_run(maybes, ["-install"])

or idk, maybe it should be locked behind a flag and/or be a CI only test

Maybe it'd be better as a build flag so the CI can say "run postinstall after install", also giving users the choice. And making the installation a single step (with he optional flag) for those who used to use the exe installers for that. (using https://pip.pypa.io/en/stable/cli/pip_install/#cmdoption-C once #2208 and #2349 are resolved)

Avasam avatar Oct 12 '24 17:10 Avasam

hmmm, can't find DLL in CI. Is it because of the --user install or am I missing something ?

Avasam avatar Oct 12 '24 17:10 Avasam

The test scripts are run from the source dir, but the post-install script needs to be run from the installed dir. ie, it should probably be in the CI tasks rather than hacked into the test script. I think just executing it is fine - no need to run tests again or anything, just the fact it doesn't fail is enough. I'm not that worried about the installed state being changed as I'm fairly sure the wheels are created from the build dir and isn't going to touch the installed state.

I'm also not too worried about this for the release - have you been able to manually verify the postinstall script now works?

mhammond avatar Oct 12 '24 18:10 mhammond

the post-install script needs to be run from the installed dir. ie, it should probably be in the CI tasks rather than hacked into the test script.

Alright, that makes sense I'll update that.

I think just executing it is fine - no need to run tests again or anything, just the fact it doesn't fail is enough

Yeah I just want a "smokescreen" test, ie: does it even run

have you been able to manually verify the postinstall script now works?

As far as I can tell, yeah. I applied the same fix as #2381 in my installed site-packages on 307 and I'm no longer getting circular import errors)

Avasam avatar Oct 12 '24 19:10 Avasam

I finally figured out the issue! The -destination argument of pywin32_postinstall is trying to default to the location of the pywin32 installation, but was incorrectly assuming it'll always be in the global platlib location (C:\hostedtoolcache\windows\Python\3.8.10\x64\Lib\site-packages). Which isn't always true as demonstrated in our CI, where we install in the user location C:\Users\runneradmin\AppData\Roaming\Python\Python38\site-packages). Specifying -destination explicitly did the trick.

Surely there's a better default we can use, but I'll leave that to a different PR, and create an issue to track it.

Avasam avatar Jan 05 '25 19:01 Avasam