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

[Eli Schwartz]

On 5/19/19 3:52 PM, Jouke Witteveen wrote:
> 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.

That is a very interesting point. Would you like to submit a patch
modifying the manpage to suggest using .pkg.tar "if you want to disable
compression"?

-- 
Eli Schwartz
Bug Wrangler and Trusted User

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 1601 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/pacman-dev/attachments/20190519/bd569748/attachment.sig>