[pacman-dev] [PATCH] makepkg: Add '--nocompress' syntactic sugar

[Jouke Witteveen]

On Sun, May 19, 2019 at 9:31 PM Eli Schwartz <eschwartz at archlinux.org> wrote:
>
> On 5/19/19 3:20 PM, Jouke Witteveen wrote:
> > On Sun, May 19, 2019 at 5:01 AM Eli Schwartz <eschwartz at archlinux.org> wrote:
> >>
> >> On 5/18/19 8:01 AM, Jouke Witteveen wrote:
> >>> On Sat, May 18, 2019 at 1:54 PM Allan McRae <allan at archlinux.org> wrote:
> >>>>
> >>>> On 18/5/19 9:34 pm, Jouke Witteveen wrote:
> >>>>> Compression is a waste of time when building a package for local
> >>>>> installation. It could already be suppressed easily, but not yet
> >>>>> via an argument to makepkg.
> >>>>
> >>>> As it could already be suppressed easily (makepkg.conf and with an
> >>>> environmental variable), we don't need an option.
> >>>
> >>> makepkg.conf is of course unsuitable for one-off package building
> >>> without compression. The environmental variable has different
> >>> ergonomics and discoverability.
> >>
> >> I agree with Allan that this is unnecessary.
> >>
> >> My assumption is that anyone interested in disabling compression will be
> >> interested in doing so for many packages, not just one -- the use case
> >> would presumably be for building custom packages for personal use, where:
> >> - disk space is usually not an issue
> >> - bandwidth is unaffected since it is never traveling over the network
> >> - the cpu cycles and wait time for compression are thus not worth it
> >>
> >> And in this case, it is equally true that you never want compression,
> >> whether you only intend to build one package ever, or not.
> >>
> >>> With the proposed patch, the semantics are simplified: Use
> >>> --nocompress if you want no compression, versus remembering to use
> >>> PKGEXT and what value to set it too.
> >>
> >> The semantics become more complicated, because there are now three
> >> places to disable compression rather than two, and one of those places
> >> *only* disables compresion, while the other two are also useful to
> >> choose a compression type.
> >>
> >> IMHO the current semantics are clearer, because the two places that it
> >> can be set are both defined as
> >> "choose a compression type, including 'no compression'"
> >>
> >>> The patch pulls this logic inside
> >>> makepkg and adds it to the documentation (manpage, bash-completion,
> >>> and --help). I'd say there is some definite value in the few extra
> >>> lines this adds.
> >>>
> >>> Anyway, thanks for your quick response!
> >>> - Jouke
> >>
> >> I don't think the argument should be about the number of lines it adds.
> >> Useful functionality should not be rejected for size issues except in
> >> extreme cases, and makepkg can definitely afford 284 bytes.
> >>
> >> The issue I see is the increased surface this adds, the fact that the
> >> --help text becomes longer (it already has enough options that even the
> >> short help text does not completely fit on my screen), the minor
> >> cognitive burden of deducing which of several ways to specify a thing
> >> will take precedence if all of them are being used.
> >>
> >> I'm not saying this is a major blocker, but it is definitely a
> >> consideration of some sort, and I don't currently believe there is a
> >> compelling argument that adding this syntactic sugar for an existing
> >> feature is sufficiently useful to justify yet another option.
> >>
> >> ...
> >>
> >> Implementation-wise, if I was passing a command-line option I would
> >> expect it to work always, no matter what, with absolutely no exceptions.
> >> For example, if I pass --nocheck, I expect that to override
> >> makepkg.conf's BUILDENV setting, and in fact, makepkg has a separate
> >> state variable to determine whether it is explicitly overridden on the
> >> command line ($RUN_CHECK) or set via makepkg.conf (the check_buildenv()
> >> function):
> >>
> >> if [[ $RUN_CHECK = 'y' ]] || { ! check_buildenv "check" "n" && [[
> >> $RUN_CHECK != "n" ]]; }
> >>
> >> Where this matters is for PKGBUILDs that have (unwisely) set this in the
> >> PKGBUILD itself. No matter how odd it is to do so, it should still be
> >> graded with the priority of a makepkg.conf setting, and not override an
> >> explicit command line option. This assumption holds true for
> >> --check/--nocheck, and does not hold true for your suggested --nocompress.
> >>
> >> It also matters if someone sets an environment override, e.g.
> >>
> >> makepkg CC=clang
> >>
> >> makepkg has little-known behavior modeled after `make` in this regard.
> >> Your suggested implementation relies on the fact that makepkg performs
> >> option parsing before sourcing makepkg.conf, and that it backs up and
> >> restores the PKGEXT variable from the environment in order to preserve
> >> it. This results in the --nocompress option acting identical to setting
> >> the environment variable. But it is worth less than `extra_environment`.
> >> While I do not "expect" people to use the very confusing command line
> >> invocation:
> >>
> >> makepkg --nocompress PKGEXT=.pkg.tar.xz
> >>
> >> I would prefer that if we were to do this, we be consistent in how we
> >> handle it.
> >>
> >> --
> >> Eli Schwartz
> >
> > Wow, thanks for this detailed response! This helps a lot with my understanding.
>
> No problem! :)
>
> Have I convinced you that this is not needed, or do you still feel it
> has use and wish to continue to make the case for it or something
> similar (incorporating my implementation notes)?
>

I am convinced :-).
However, I do now think that it would maybe have been good to make
PKGEXT=.pkg.tar
the default in makepkg.conf, since local installation of AUR packages
is probably the most common usecase of makepkg.

Changing it now makes no sense as it will probably be too disruptive.
The comment ("Do not modify unless [etc.]") could be improved to
something more helpful, though.

Regards,
- Jouke