On 5/19/19 3:52 PM, Jouke Witteveen wrote: > On Sun, May 19, 2019 at 9:31 PM Eli Schwartz <[email protected]> wrote: >> >> On 5/19/19 3:20 PM, Jouke Witteveen wrote: >>> On Sun, May 19, 2019 at 5:01 AM Eli Schwartz <[email protected]> >>> wrote: >>>> >>>> On 5/18/19 8:01 AM, Jouke Witteveen wrote: >>>>> On Sat, May 18, 2019 at 1:54 PM Allan McRae <[email protected]> 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
signature.asc
Description: OpenPGP digital signature
