Sender: =?UTF-8?Q?Timo_Ter=C3=A4s?= <timo.teras@gmail.com>
Date: Fri, 28 Jun 2013 13:21:45 +0300
From: Timo Teras <timo.teras@iki.fi>
To: Dubiousjim <dubiousjim@gmail.com>
Cc: alpine-devel@lists.alpinelinux.org
Subject: Re: [alpine-devel] [PATCH 00/15] apk-tools UI fixes/refinements
Message-ID: <20130628132145.52918d9d@vostro>
In-Reply-To: <cover.1372361242.git.dubiousjim@gmail.com>
References: <cover.1372361242.git.dubiousjim@gmail.com>
Precedence: list
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Thu, 27 Jun 2013 15:28:45 -0400
Dubiousjim <dubiousjim@gmail.com> wrote:

> Some months ago, I patched apk-tools in various ways to (in my view)
> improve the UI and also to provide a logging mechanism. I've been
> using these in practice on my local machine. It's about time to
> rebase them against the current apk-tools git repo, and see if they
> can be incorporated upstream.
> 
> This set of patches has the UI tweaks. It'll take me a bit longer to
> rebase/test the logging patch against the current apk-tools
> sourcebase.
> 
> For the most part, these UI tweaks don't introduce anything that will
> break existing behavior---except in 1 place (or perhaps 1-and-a-half
> places), which I'll describe below. However, I have introduced some
> new short/long option flags for existing functionality, and in some
> places have declared the old flags "deprecated." In the current
> implementation, all that deprecation amounts to is that the usage
> message recommends the new flag; no warnings are output when the old
> flag is in fact used.
> 
> 1. One set of changes is that we move some options that are now
> listed in the "generic options" usage block, but which don't in fact
> apply to all applets, into the applet-specific option set for the
> applets that use them. This applies to -U/--update-cache, --purge,
> and --clean-protected.

Correct. These basically apply to applets that eventually commit
changes to database.

But as mentioned I'd rather have these as an option group. And keep the
help text in single place along with the code to handle these options.
Either have a "option_groups" bitfield and implement these in the
general code, or add some functions to "install" or "register" these
shared extra options per-applet.

> 2. A second change we introduce is that we accept "apk mkindex" to
> mean what "apk index" already means, and prefer the former. This
> makes its behavior more memorable,at least for me, and stops me from
> wondering why functions like "apk version -l" aren't underneath the
> "apk index" applet instead.

Sound reasonable.

> A lot of changes try to impose some more consistency on the option
> flags, especially the short option flags, either with other applets
> or with familiar POSIX utilities.
> 
> 3. One of those sets of changes concerns the various flags for
> recursing upwards or downwards in the dependency tree.
> 
> Let me frame my overview of those changes with this observation:
> POSIX tools mostly use -R for recursive operations (consider chmod,
> chown, chgrp, ls, cp, rm). cp also accepts -r, sometimes with
> equivalent behavior to -R. But not all implementations can be relied
> on to treat "cp -R" and "cp -r" equivalently; and where they diverge,
> it's usually the first that's wanted. (See the OS X cp.1 manpage.) rm
> also accepts -r, and can be relied on to treat "rm -R" and "rm -r"
> equivalently. But the IEEE/Open Group's rm.1p manpage recommends
> using "rm -R", for consistency with other utilities. The Gnu *grep
> utilities (which are also the defaults on some BSD systems) also
> treat -R and -r equivalently. As it happens, BusyBox *grep only
> honors -r, but I will be submitting a patch to BusyBox upstream to
> also accept -R, as the Gnu versions do.
> 
> The only remaining familiar utilities to ONLY accept -r for recursive
> functionality are diff and rsync. On the other hand, a whole range of
> utilities use -r for other purposes (consider date, touch, ls, sort,
> Gnu and BusyBox sed, xargs, vi, and so on).
> 
> So the strongest existing pattern here seems to be to use "-R" to mean
> "recurse".
> 
> But what does a recursive operation in a dependency tree amount to?
> We could mean "also operate on the specified package's dependencies."
> Let's use --depends as the long-form name of that option. (As
> apk-tools sometimes does; currently it also sometimes uses
> --recursive here instead.) Or we could mean "also operate on the
> packages that depend on the specified package." Let's use --rdepends
> as the long-form name of that option. (For "reverse-depends", not
> "recursive depends.") It would be confusing to have -R be the
> short-form option for "--depends", and some other letter be the
> short-form option for "--rdepends". So I propose we consistently use
> "-R" in the apk-tools namespace to mean "--rdepends", that is, to
> operate recursively UPWARDS in the dependency graph (towards packages
> that have more dependencies). Sometimes apk-tools currently uses "-d"
> as the short-form option for the other direction (--depends). But
> other times apk-tools uses -d differently. For consistency with -R, I
> propose we use "-D" here instead.
> 
> So in general:
> 
>   -D/--depends means to also apply an operation to the packages that
> the specified package depends on, that is, downwards in the dependency
>   graph
> 
>   -R/--rdepends means to also apply an operation to the packages that
>   depend on the specified package, that is, upwards in the dependency
>   graph

Merging these flags has been something I've been thinking as well. I
definitely agree that they need a redo. See below for more.

> These patches roll out that change across the apk applets and usage
> messages, while still accepting the old short and long options too,
> where these exist. We just declare some of the old options as
> "deprecated" in the usage messages.
> 
> There's only one place where this requires us to introduce a breaking
> change: currently "apk info -R" is interpreted as "apk info
> --depends", but these changes will make it be interpreted in the
> other direction, as "apk info --rdepends". I hope this small
> deviation from existing behavior is tolerable.
> 
> One awkwardness I haven't resolved is that in most cases, both the -D
> and -R options are recursive: -D operates on your dependencies, and
> their dependencies, and so on. However in "apk info", I think -D and
> -R only apply to a single level of dependencies: only your direct
> dependencies, and so on. It'd be nice if we could make this work
> recursively too (and perhaps had a flag to limit the recursion to
> only a single level). But that isn't implemented here.
> 
> 
> 4. Another option-related change is that we now let "apk search -x"
> mean "apk search -e/--exact", and prefer the former. This is
> consistent with the -x flag on grep, and also steps away from the
> conflicting meaning of "-e" in "apk info".

Sounds ok.

> There are some other changes as well, which should be noted in the
> individual commits. Here I'm just trying to give the high-level
> overview, not an exhaustive description.
> 
> Here are some further ideas for changes that I DON'T try to implement
> here, but I submit for consideration/discussion.
> 
> * As with "apk index" -> "apk mkindex", I'd find it easier if we
> aliased "apk update" to something less similar to "apk upgrade":
> perhaps "apk pull"?

Not so sure about this one. Then again, "-U" flag in the applets where
it takes effect, does what 'update' does alone. Need to think this a
bit.

> * I'm not sure how things stand with the current version of
> apk-tools. But I know that at least with 2.3.4ish versions, "apk fix"
> would ignore /etc/apk/cache. That is, if I have foo-1.0 in the cache,
> but foo-1.1 is available on the repositories, then even if I
> explicitly request "apk fix -r foo" (as opposed to "apk fix -u foo"),
> I'd get upgraded to foo-1.1. This is surprising and undesirable.
> Could we change solver.c to prefer packages in /etc/apk/cache in such
> cases?

This should have been fixed with the new solver code.

> * It'd be nice if "apk version" also displayed which repository an
> upgrade would come from.

It shows the pinning. For the exact repositories, one can now use "apk
policy <pkgname>". Do note that "apk version" says the latest version,
not the version it would upgrade to.

Apk version is a wee bet overloaded, as it can do many unrelated
things. Might be time to split it.

> * for backwards compatibility, "apk fetch -R" is still honored,
> though "apk fetch -D" is now the preferred form. It may be confusing
> to have "apk fetch -D" and "apk fetch -R" have the same
> functionality, so we could consider immediately deprecating "apk
> fetch -R" more forcefully, for example by issuing a warning message.
> 
> * As I mentioned above, it'd be nice if "apk info -D/-R" recursed
> over more than a single level of dependencies, or at least if there
> were an option to do so.
> 
> * Is "--overlay-from-stdin" really a generic option, or like
> --clean-protected and --purge does this option too only apply to some
> of the applets?

It applies like --clean-protected. Applets that commit things to
database use it. In practice it is only used on tmpfs root setups
during initramfs bootstrap.

> * One of the patches below deprecates short-form "apk info -i", for
> reasons described in the commit. Now, the following proposal is
> probably just my own taste. But I'd suggest deprecating a bunch more
> of the short-form options: "apk -p", "apk fetch -s", "apk index -x",
> "apk upgrade -a", and all of "apk info -IPswt". In the first two
> cases, because these are unusual actions anyway and the short-form
> options aren't very memorable. It's easier and clearer in practice to
> just use the long form; and then the apk-tools option namespace will
> be clearer. In the third case ("apk index -x"), in part because this
> conflicts with the different meaning introduced above for "-x" wth
> "apk search." In part also because there's the confusing variety of
> "apk -X/--repository", "apk version -I/--indices", "apk index
> -x/--index". In the fourth case ("apk upgrade -a"), because it
> conflicts with the different meaning for "-a" ("--all") in some other
> apk commands. In the case of the "apk info" options: -I already has a
> different meaning for "apk version", plus I already suggested not
> using its relative "-i". Additionally, "-I" and "-P" are different in
> the character of their behavior from other capital letter short-form
> options for "apk info" (-L, -W, -R, -D). It seems clearer to just use
> the long forms for "-I" and "-P"; plus these are going to be rare
> actions in any case. For "apk info -swt": it's hard to remember which
> output-selecting options have short-forms (and what they are) and
> which don't. It'd be cleanest to just honor "-d" for "--description"
> and have the other output options be long-form only. Also, "-s" for
> "apk info" would conflict with the meaning of "-s" for "apk fetch",
> if we chose to retain that.

Agreed, these could do a little rework. Most of apk flags are what
they are due to historical reasons (from the shell based apk-tools
1.x), and the rest just "evolved" when needed.

Redesigning the current set of flags to be "unified" would be a good
thing.

> * I don't think I understand the difference between "apk add foo" and
> "apk add -u/--upgrade foo". Perhaps I asked about this before, but if
> it was ever explained to me, it's not something I've kept track of.

apk add always adds the given files to 'world'. If foo is already
installed, it is not upgraded by default (unless dependencies so
required).

Thus, if foo is installed: apk add -u foo, will upgrade it; apk add foo
will not but just ensures it is in world so that future apk del
pkg-using-foo will not cause it to be deleted due to
becoming unneeded dependency.

> * "apk info" and "apk search" have different output styles, and
> interact with "-q" and "-v" differently. Here is the current behavior:
> 
>     apk info:     list all installed packages, without -1.0-r0
> suffixes apk info -v:  list all installed packages, with -1.0-r0
> suffixes apk info -vv: list all installed packages, with -1.0-r0
> suffixes, also list descriptions
>     apk info -v [moreoptions] packagename: machine-friendly output
> 
>     apk search substring:    list matching packages, with 1.0-r0
> suffixes apk search -q substring: suppress -1.0-r0 suffixes
>     apk search -v substring: list matching packages plus
> descriptions, on one line
> 
>   I observe also that "apk info" always seems to ignore "-q", and
> "apk version -q" is, perversely, much more verbose than "apk version".
>   
>   This is all confusing and hard to remember. It'd be nice if we could
>   standardize on a single mapping between output formats and options,
> perhaps following the pattern of "apk search".

It might be also possible now to merge 'search' and 'info' applets
together. The fundamental difference is that info works with installed
packages, and search works with available packages - thus info does
have some extra information output that is available only with
installed packages. But all in all, most of the functionality is shared.

I think we'll need to think out what we want for:
 - the new set of flags
 - the new output format for info/search
 - if any other changes needed: e.g. split 'apk version' to two or
   three applets
 - then break things in one go and do apk-tools 2.5 or 3.0

I will likely commit the minor clean ups to master before next tag. But
the bulk of this, and the logging patch set would likely wait for 3.0.

The reason is we're planning to backport apk-tools 2.4.x to our -stable
alpine releases and do not want to break things there.


---
Unsubscribe:  alpine-devel+unsubscribe@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---