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

Distribution now bloated with libraries - why? and please add an option for dynamic link of libonig/libjq #78

Open
mandree opened this issue Sep 23, 2022 · 6 comments

Comments

@mandree
Copy link

mandree commented Sep 23, 2022

I see that as of 1.3.0 oniguruma and jq are now part of the tarball. I checked changelog and the commit log entry, but...

WHY?

The distribution size went up from 70 kB to almost 3 MB and - myself being a package of py-jq as we call it for FreeBSD - I am actively patching out the build because we have separate onig and jq packages so will not want to rebuild for just installing jq Python bindings.

I am so bold to ask you to revert the change and explain the issue you were trying to solve so that somebody can help find a better solution than just dumping dependencies into the tarball.

@mwilliamson
Copy link
Owner

Installing from the sdist previously downloaded the source for jq and oniguruma, which meant that there was an installation time dependency on all that entails (having a network connection, the sites hosting the source being up, and so on). By having those included in the sdist, the package can now be installed, for instance, without an Internet connection.

I am actively patching out the build because we have separate onig and jq packages so will not want to rebuild for just installing jq Python bindings.

I'm not sure what you were doing before, but this package has always used its own copies of jq and onigurama -- the only difference is whether they're downloaded at install time, or pre-downloaded and included in the sdist.

@mgorny
Copy link

mgorny commented Sep 24, 2022

Just please add an option to use system libs instead.

@mandree
Copy link
Author

mandree commented Sep 24, 2022

Installing from the sdist previously downloaded the source for jq and oniguruma, which meant that there was an installation time dependency on all that entails (having a network connection, the sites hosting the source being up, and so on). By having those included in the sdist, the package can now be installed, for instance, without an Internet connection.

Which a package provider should not need to be concerned with. jq is a stable package (not to call it unmaintained) provided by many distributions, and oniguruma seems to be slow moving and compatible, so also "none of a jq wrapper's concern", and if I were to not have jq installed, I would not need the wrapper either.

We have Python standard packaging and wheel building and whatever other facilities and distributions to download/install requisites and you can also provide a download "shopping list" if you want.

No need to bloat a small package from 70 kBytes to 30 or 40 times its size. If you still feel the need to provide a "batteries included" package, then please ALSO provide the prior small package we used to have until 1.2.3, and possibly even without the download/build stuff, just move that section into documentation such as "in September 2022, these versions of onig' and jq have been tested and are known to work ... + URL".

I am actively patching out the build because we have separate onig and jq packages so will not want to rebuild for just installing jq Python bindings.

I'm not sure what you were doing before, but this package has always used its own copies of jq and onigurama -- the only difference is whether they're downloaded at install time, or pre-downloaded and included in the sdist.

We have always, in the FreeBSD ports/packages context, been installing jq and oniguruma as separate packages, depend or py-jq package as we call it, on them as library requisites, and were using those (as .so libs, not static), and I have been actively disabling the two lines that called the "local download/build" features. This is overall a much more economic use of everything, maintainership hours, system memory, download sizes, whatnot.

I know this has worked before 1.3.0, for certain, because our builders do not have network access after downloading and checksumming the original 70 kBytes of jq.py and I have never received a single fallout report message from our automated package builders because jq or oniguruma were messed up.

It is just redundant. The world needs a small "bindings-only" py-jq package.

FTR, this is FreeBSD's patch (repo access though CGit here), which does these things:

  1. prevent the local build
  2. switch to linking to dynamic onig/jq to avoid setting up technical debt in the form of tight couplings to whatever library link statically.

Michał (@mgorny) is also proposing an option to link dynamically to system libonig.so and libjq.so. While here, let me edit the issue's title to also encompass that static link. (spelling of Michał's name corrected in my edit - sorry)

--- setup.py.orig	2022-09-19 15:51:09 UTC
+++ setup.py
@@ -36,8 +36,6 @@ class jq_build_ext(build_ext):
     def run(self):
         if not os.path.exists(_dep_build_path(".")):
             os.makedirs(_dep_build_path("."))
-        self._build_oniguruma()
-        self._build_libjq()
         build_ext.run(self)
 
     def _build_oniguruma(self):
@@ -87,11 +85,7 @@ jq_extension = Extension(
     "jq",
     sources=["jq.c"],
     include_dirs=[os.path.join(jq_lib_dir, "src")],
-    extra_link_args=["-lm"],
-    extra_objects=[
-        os.path.join(jq_lib_dir, ".libs/libjq.a"),
-        os.path.join(oniguruma_lib_install_dir, "lib/libonig.a"),
-    ],
+    extra_link_args=["-lm", "-ljq", "-lonig"],
 )
 
 setup(

@mandree mandree changed the title Distribution now bloated with libraries - why? Distribution now bloated with libraries - why? and please add an option for dynamic link of libonig/libjq Sep 24, 2022
@rljacobson
Copy link

I think this is a very sensible request, though I object to the tone with which it was made. It is pretty standard to not bundle dependencies in the Python module ecosystem, or else to put it behind a feature flag. Of course, you can find exceptions.

If I understand the issue correctly, #81 is caused by this issue, and there is already a pull request, #82, that solves it. In the parlance of our times, winner winner chicken dinner.

@813gan
Copy link

813gan commented Sep 6, 2024

I second this request.
After XZ drama it's clear all binary blobs are bad.

@813gan
Copy link

813gan commented Sep 6, 2024

PYQJ archived this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants