Install actually working version of the project#228
Conversation
|
This should allow #226 regardless of whether we implement zeek/zeek#5597 or not. |
25afcbd to
ed8ddc2
Compare
41a1c10 to
34210ea
Compare
|
I opened zeek/zeek#5604 to testdrive integration of this into Zeek, and it seems to work with very minor adjustments. |
34210ea to
81874d5
Compare
There was a problem hiding this comment.
Super interesting approach — I hadn't heard of shiv, or the fact that Python can execute a __main__.py in a ZIP file. I still need to try it out in a Zeek tree, but I'm excited.
One question: I know we have (limited, but legit) use cases where the Zeek installation happens airgapped. If we produce a source tarball for the Zeek distro, will the resulting ZIP be contained in it, or will the configure stage then go ahead and pip-download the dependencies? It's not a showstopper — right now the dependencies need to get onto such systems after all, too. Just trying to understand.
Basically, I wonder if we could keep the dependencies functioning "normally" when the user wants to, but then it's also up to the user to ensure the dependencies are satisfied (via system packages, system--wide pip install, etc).
|
|
||
| * Python 3.6+ | ||
| * git: https://git-scm.com | ||
| * GitPython: https://pypi.python.org/pypi/GitPython |
There was a problem hiding this comment.
I'm not sure we should remove this here — these are still dependencies, we just handle them differently now?
There was a problem hiding this comment.
Unless we want this to be some SBOM (which we then should keep up to date more diligently) I'd argue this should go away. I'd also argue that this PR did not really change our approach to dependencies since the authoritative source was always pyproject.toml or maybe the slightly out of sync requirements.txt.
81874d5 to
e443407
Compare
Yeah, with this PR we would produce a tarball which cannot be built in airgapped environments (the configure step should still succeed). That said, I am curious how creating a Zeek environment with working zkg would look today since fixing up the installation by making the Python dependencies available would still require internet access; AFAICT we do not document this at all. I suspect most such airgapped installation already disable zkg. To make this work we could vendor both build and runtime dependencies and then hook them into the build with
I'd argue that with the introduction of virtualenv and pip circa 2008 installing into the system has become not "normal". Distributions are pulling the plug on this as well with now requiring |
|
Yeah. I also like the line of thought during our last conversation: the airgapped use-case brings up the question of how the Python dependencies ended up on the system in the first place, so a custom, independent install of zkg (via a I just tried it out locally in a Zeek tree. This is what looks like during configure: Pretty cool. I'm pretty much ready to approve, just two more thoughts:
|
e443407 to
57ff7e8
Compare
Yeah, Having
The |
While this project has a perfectly fine `pyproject.toml` which allows installing it with `pip`/`pipx`, we also shipped a `CMakeLists.txt` to make the project installable when shipped as part of a bigger CMake project like Zeek. The `install` rules we had were bare bones since they only installed the top-level `zkg` script and the `zeekpkg` module it depends on, but e.g., not any dependencies. It likely would also have broken for potential future tweaks to `pyproject.toml` like switching to another build backend. This patch reworks our CMake install rules to install a fully functioning version of this project instead. This allows users to use it directly without having to install and manage dependencies themself. This removes potential for user error (e.g., version mismatches) and should also allow us to use dependency more freely/aggressively. We accomplish this with `shiv` which leverages PEP441 to create self-contained environments in zip files which can be loaded by any Python interpreter >=python-3.4. Users invoke a small wrapper we create which forwards to the top-level `zkg` script.
We would previously potentially leave CMake variables in the zkg script since they would only get expanded by a CMake build, but not by e.g., `pip`. The CMake install rewrite in the previous patch also did not expand them anymore. This patch adds that lost functionality back into the wrapper script which we now have and which is only used by a CMake install.
This project does not do any compilation, so checking for a C compiler (which is the default if no language was specified in `project`) is just doing busywork.
This bumps the minimum CMake version to the one required by e.g., Zeek.
We would previously document dependencies which are already expressed programmatically in `pyproject.toml`, so this added information not useful for readers, but also introduced potential for things to get out of sync (which they had already). Just drop anything already expressed in the project config.
57ff7e8 to
fea4017
Compare
While this project has a perfectly fine
pyproject.tomlwhich allowsinstalling it with
pip/pipx, we also shipped aCMakeLists.txttomake the project installable when shipped as part of a bigger CMake
project like Zeek. The
installrules we had were bare bones since theyonly installed the top-level
zkgscript and thezeekpkgmodule itdepends on, but e.g., not any dependencies. It likely would also have
broken for potential future tweaks to
pyproject.tomllike switching toanother build backend.
This patch reworks our CMake install rules to install a fully
functioning version of this project instead. This allows users to use it
directly without having to install and manage dependencies themself.
This removes potential for user error (e.g., version mismatches) and
should also allow us to use dependency more freely/aggressively.
We accomplish this with
shivwhich leverages PEP441 to createself-contained environments in zip files which can be loaded by any
Python interpreter >=python-3.4. Users invoke a small wrapper we create
which forwards to the top-level
zkgscript.