jq.py icon indicating copy to clipboard operation
jq.py copied to clipboard

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

Open mandree opened this issue 3 years ago • 6 comments

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.

mandree avatar Sep 23 '22 20:09 mandree

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.

mwilliamson avatar Sep 23 '22 21:09 mwilliamson

Just please add an option to use system libs instead.

mgorny avatar Sep 24 '22 07:09 mgorny

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 avatar Sep 24 '22 09:09 mandree

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.

rljacobson avatar Apr 10 '23 20:04 rljacobson

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

813gan avatar Sep 06 '24 11:09 813gan

PYQJ archived this.

813gan avatar Sep 06 '24 12:09 813gan