Date: Fri, 28 Jun 2013 12:37:47 -0400
From: Dubiousjim <dubiousjim@gmail.com>
To: alpine-devel@lists.alpinelinux.org
Subject: Re: [alpine-devel] futher apk ideas (WAS: [PATCH 00/15] apk-tools UI
 fixes/refinements)
Message-ID: <20130628163747.GB1712@zen.nyu.edu>
Mail-Followup-To: alpine-devel@lists.alpinelinux.org
References: <cover.1372361242.git.dubiousjim@gmail.com>
 <20130628154631.4d18107d@ncopa-desktop.alpinelinux.org>
Precedence: list
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20130628154631.4d18107d@ncopa-desktop.alpinelinux.org>
User-Agent: Mutt/1.5.21 (2010-09-15)

Thanks Natanael and Timo for your comments. I'm glad we're mostly on the
same page about these things. Much of what I threw out was just to
invite reactions.

I'll have a look at what you've already committed and rebase the commits
Timo asked me to separate out, shortly. This is just to respond to some
of your comments.

I'll split out the discussion of -R/-D and the logging to separate
emails.


On Fri, Jun 28, 2013 at 01:21:45PM +0300, Timo Teras wrote:
> 
> 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.

Makes perfect sense.


On Fri, Jun 28, 2013 at 01:21:45PM +0300, Timo Teras wrote:
> 
> 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

Right, that makes perfect sense too.


On Fri, Jun 28, 2013 at 11:13:33AM +0200, Natanael Copa wrote:
> >  static struct apk_applet apk_add = {
> >  	.name = "add",
> > -	.help = "Add/update PACKAGEs to main dependencies and install them",
> > +	.help = "Add PACKAGEs to main dependencies and install (or upgrade) "
> > +		"them, while ensuring that all dependencies are met",
> >  	.arguments = "PACKAGE...",
> >  	.open_flags = APK_OPENF_WRITE,
> >  	.context_size = sizeof(struct add_ctx),
> 
> it could have been: (or upgrade, if -u is given)

How about this comment message for "apk add": "Register package in
'world' and install it, if necessary, while ensuring that all
dependencies are met"

Perhaps adding, either there or on the help for the "-u" flag, something
like "If the package is already installed, `apk add` ignores any updates
that may be available, unless the `-u` option is provided"


On Fri, Jun 28, 2013 at 11:19:03AM +0200, Natanael Copa wrote:
> > @@ -183,8 +183,8 @@ static int ver_main(void *pctx, struct apk_database *db, struct apk_string_array
> >  
> >  static struct apk_option ver_options[] = {
> >  	{ 'I', "indexes",	"Print description and versions of indexes" },
> > -	{ 't', "test",		"Compare two given versions" },
> > -	{ 'c', "check", 	"Check if the given version string is valid" },
> > +	{ 't', "test",		"Compare two given versions, output '<' '=' or '>'" },
> > +	{ 'c', "check", 	"Check the given version strings, output any that are invalid" },
> >  	{ 'a', "all",		"Consider packages from all repository tags" },
> >  	{ 'l', "limit",		"Limit output to packages with status matching one of LIMCHARs",
> >  	  required_argument, "LIMCHARs" },
> 
> shouldn't it be a comma (,) in there?
> 
> ...output '<', '=' or '>'

If you like. I'll have a look to see whether this is a commit you want
me to re-supply.


> > @@ -192,7 +192,8 @@ static struct apk_option ver_options[] = {
> >  
> >  static struct apk_applet apk_ver = {
> >  	.name = "version",
> > -	.help = "Compare package versions",
> > +	.help = "Compare package versions (in installed database vs. available) "
> > +		"or do tests on literal version strings (with or without -rN suffixes)",
> >  	.open_flags = APK_OPENF_READ,
> >  	.context_size = sizeof(struct ver_ctx),
> >  	.num_options = ARRAY_SIZE(ver_options),
> 
> Can we drop the "(with or without -rN suffixes)" part? it looks like -r -N options...

If you like. Is there some other way we might communicate the same
thing? Otherwise there's no way for someone to figure this out except by
experimenting. Which isn't sooo bad.


On Fri, Jun 28, 2013 at 11:31:01AM +0200, Natanael Copa wrote:

> >  	if (apk_flags & APK_SIMULATE) {
> > -		apk_warning("This simulation is not reliable as apk-tools upgrade is available.");
> > +		apk_warning("This simulation is not reliable if apk-tools upgrade is available.");
> >  		goto ret;
> >  	}
> >  
> 
> I think this warning will/should only be displayed when it would
> perform a self-upgrade. Proper warning would probably be:
> 
> "This simulation is not reliable as apk-tools or its dependencies would be upgraded".

On Fri, Jun 28, 2013 at 01:55:54PM +0300, Timo Teras wrote:
> On Fri, 28 Jun 2013 11:31:01 +0200
> Natanael Copa <ncopa@alpinelinux.org> wrote:
> 
> > I think there have been bugs recently that has made apk perform the
> > self upgrade at 'random'. The self-upgrade should only happen when
> > apk-tools or one of its dependencies are upgraded. Maybe it should
> > self-upgrade only when apk-tools itself needs upgrade. (dependencies
> > should be ABI compat so they should not have any effect on the result)
> 
> Yes, the new solver code is better at this. There is still situations
> when dependencies need to get upgraded. E.g. new apk-tools depends on
> libc>=something. And that needs libc upgrade to get satisfied.

Right, the version I was originally developing these patches against
would always give the warning, that's why I reworded it. I hadn't
noticed that the logic had now changed.

I like the idea that Natanael and Timo were discussing of just using
installed dependencies when self-upgrading---if the ABIs permit.


On Fri, Jun 28, 2013 at 01:03:19PM +0300, Timo Teras wrote:
> On Fri, 28 Jun 2013 11:35:09 +0200
> Natanael Copa <ncopa@alpinelinux.org> wrote:
> 
> > On Thu, 27 Jun 2013 15:28:51 -0400
> > Dubiousjim <dubiousjim@gmail.com> wrote:
> > 
> > > ---
> > >  src/add.c     | 4 ++++
> > >  src/apk.c     | 4 ----
> > >  src/dot.c     | 4 ++++
> > >  src/fetch.c   | 4 ++++
> > >  src/fix.c     | 4 ++++
> > >  src/info.c    | 4 ++++
> > >  src/search.c  | 4 ++++
> > >  src/upgrade.c | 4 ++++
> > >  src/ver.c     | 4 ++++
> > >  9 files changed, 32 insertions(+), 4 deletions(-)
> > 
> > Since there are many applets using same option, I wonder if we could
> > join them somehow?
> > 
> > It might not matter since I think the gcc optimizer will store only
> > one copy of the identical string constants.
> 
> I think it should do this automatically. But I really don't like
> duplicating the code how to handle each option in all the applets.
> 
> I do like the idea of pushing these options down the applets that
> actually use them. So seems we need some sort of "option group" support.

I understand where you're coming from. Pragmatically, though, the
option-handling code is just a single bit-setting in each case.
Replacing this with a duplicate bit-setting to indicate the "option
group" doesn't seem to gain much.

On the other hand, maybe there are broader benefits to introducing
option groups. Also, it's true that there is the duplicated usage
message.


On Fri, Jun 28, 2013 at 03:46:31PM +0200, Natanael Copa wrote:
> On Thu, 27 Jun 2013 15:28:45 -0400
> Dubiousjim <dubiousjim@gmail.com> wrote:
> > 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"?
> 
> apk pull reminds me of git pull. (which will do both fetch and merge),
> but is an option yes.
> 
> other options:
>   apk (re)fresh
>   apk fetchdb
>   apk syncdb
> 
> I'm not convinced it is worth it though.
> 
> maybe a single 'apk -U'?

Right, "git pull" was the model for my suggestion. "git fetch" is the
closer analogy, but "apk fetch" is already used.

I like the idea of honoring bare "apk -U/--update-cache" the best. I
think implementing that would require somewhat more ambitious code
changes.


On Fri, Jun 28, 2013 at 02:08:09PM +0300, Timo Teras wrote:
> On Thu, 27 Jun 2013 15:28:52 -0400
> Dubiousjim <dubiousjim@gmail.com> wrote:
> 
> > Also add infrastructure for short-option-only synonyms
> 
> Care to split the infrastructure to separate commit?
> 
> I'm willing to commit that, along with the "search -x" alias and
> reordering of the options where needed, before tagging 2.4.0.
> 
> The -R / -D options thingy, along with version, index, search and info
> applet changes will be postponed.
> 
> I pushed the first four commits already with ncopa's Ack. I did minor
> change as suggested by ncopa to one of the commits, and reworded the
> commit message.
> 
> Please reword the commit messages similarly if resending the above
> mentioned commits.

Ok, will do this shortly.


On Fri, Jun 28, 2013 at 01:21:45PM +0300, Timo Teras wrote:
> On Thu, 27 Jun 2013 15:28:45 -0400
> Dubiousjim <dubiousjim@gmail.com> wrote:
> 
> > * 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.

Great!


On Fri, Jun 28, 2013 at 01:21:45PM +0300, Timo Teras wrote:
> On Thu, 27 Jun 2013 15:28:45 -0400
> Dubiousjim <dubiousjim@gmail.com> wrote:
> 
> > * 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.

Wow, I didn't realize that at all.

> Apk version is a wee bet overloaded, as it can do many unrelated
> things. Might be time to split it.

I agree. Should some of this functionality be merged with the new policy
applet? Haven't yet sorted that out in my mental map.


On Fri, Jun 28, 2013 at 01:21:45PM +0300, Timo Teras wrote:
> On Thu, 27 Jun 2013 15:28:45 -0400
> Dubiousjim <dubiousjim@gmail.com> wrote:
> 
> > * 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.

Ok, then should it also be pushed down into the applets that use it? Or the
appropriate "option group"?


On Fri, Jun 28, 2013 at 02:36:45PM +0200, Natanael Copa wrote:
> On Thu, 27 Jun 2013 15:28:45 -0400
> Dubiousjim <dubiousjim@gmail.com> wrote:
>  
> > 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.
> 
> I think this is good. I am not 100% sure but I think that the short form id integer
> might need to be universally unique.

Is that right? They can't even be duplicated between applets? I think
there is some such duplication already in the code base. But this is
just going by distant memory.


On Fri, Jun 28, 2013 at 01:21:45PM +0300, Timo Teras wrote:
> On Thu, 27 Jun 2013 15:28:45 -0400
> Dubiousjim <dubiousjim@gmail.com> wrote:
> 
> > * 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:
> 
> 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.

Again, this was just to start discussion. But I'm glad our
general inclinations here are in line.


On Fri, Jun 28, 2013 at 03:46:31PM +0200, Natanael Copa wrote:
> 
> apk fetch -s is useful to list contents of an uninstalled package:
>   apk fetch --stdout pkg | tar -zt
> 
> ...
> 
> maybe change 'apk fetch -s' to 'apk fetch -O' similar to 'tar -O' (and
> 'wget -O -')

I wasn't suggesting to eliminate the functionality, just the short-form
option name "-s". I like the "-O" short-form much better.


On Fri, Jun 28, 2013 at 02:36:45PM +0200, Natanael Copa wrote:
> On Thu, 27 Jun 2013 15:28:45 -0400
> Dubiousjim <dubiousjim@gmail.com> wrote:
>
> > 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...
> 
> I am a bit sceptic to this kind of changes. It basically means that
> there are some things that simply cannot be used in scripts...

I understand. I was trying very hard to minimize the breakage. The -R/-D
stuff needs more discussion. The "-and-a-half" thing I was referring to
above was about "apk info -i", but that seems to be currently broken
anyway (also the long form).

I was thinking we should make sure the public scripts use long-form
options (I've verified that many of them already do, except for a bunch of uses
of -q). I was thinking also that where possible, changes be introduced
like this:

        * first the new applet name/option name is enabled, and the
          old names are still honored, but the usage message suggests to
          use the new forms.
        * a bit later, the old forms are no longer listed in the usage
          message, but they are still honored, except that we also
          at the same time emit a message to stderr.

This won't break anything that doesn't capture stderr.

Also when I suggest "eliminating" short option-names, I just
mean easing them out of the recommended usage and so
on, as above, but---where possible---still honoring them.

It's just in a few places that things really invite being cleaned up in
a way that requires more breakage. The "apk info -R" is the dominant
example. I wholly agree this needs more discussion and
thought.

> ... unless we
> provide a simple way for a script to fish out the apk scripting "api"
> version.
> 
> Maybe introduce apk --api-version
> 
> if empty, then its current api, then whenever we change the scripting
> 'api' or output format, we bump the "api-version" number.
> 
> That way scripts has a way to deal with this kind of changes in future.
> like:
> 
> apk_api="$(apk --api-version 2>/dev/null)"
> : ${apk_api:=0}
> 
> if [ $apk_api -le 1 ]; then
> 	apk_mkindex="apk index";;
> else
> 	apk_mkindex="apk mkindex";;
> fi
> 
> $apk_mkindex *.apk .... etc

That's heavy-handed. Maybe it might be called for in the end, but I
hope we can find some lighter-weight and elegant solution. What I
described above wouldn't introduce any breakage with respect to "apk
index"; we'd just eventually not be documenting its existence, but would
continue to honor it.


On Thu, 27 Jun 2013 15:28:45 -0400
Dubiousjim <dubiousjim@gmail.com> wrote:

> * "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".


On Fri, Jun 28, 2013 at 01:21:45PM +0300, Timo Teras wrote:
> 
> 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.

Maybe. The main conflicts I can think of are with bare "apk info" vs
bare "apk search", and with "apk info --rdepends" vs "apk search
--rdepends". Also "apk info ..." requires an exact package name, right?
whereas "apk search ..." defaults to accepting a glob pattern.

I don't mind having the separate applets.


On Fri, Jun 28, 2013 at 03:46:31PM +0200, Natanael Copa wrote:
> 
> How about we introduce 'apk list' or 'apk ls' to list installed packages?

That's a possibility. (Replacing bare "apk info".)

> 'apk info -s' for size, like 'du -s'

That makes sense. I do value the existence of such patterns.


> Yeah, apk info output format have annoyed me too, specially the -q
> thingy since it makes it hard to use in scripts.
> 
> How about we introduce 'apk list' or 'apk ls' to list installed packages?
> 
> apk ls:		list installed packages, with 1.0-r0 suffixes
> apk ls -q:	list installed packages, without 1.0-r0 suffixes
> apk ls -v:	list installed packages, with description
> apk ls GLOB...:	list installed package which fnmatches GLOB(s)
> 
> Me and Timo as talked about a 'world' applet but a --world option could
> solve that.
> 
> apk ls --world:	list world
> 
> Then 'apk info' would be more a tool to give information about
> specified packages.
> 
> I am also missing: 'apk info --origin' to print the aport it comes from.
> 
> Maybe we could introduce 'apk info --format FORMAT' where format could
> have % modifiers that corresponds to the field in installed db:
> 
>  %C  checksum
>  %P  package name
>  %V  version
>  %A  arch
>  %S  package size (apk)
>  %I  installed size
>  %T  description text
>  %U  homepage
>  %L  license
>  %o  origin (aport name)
>  %m  package maintainer
>  %t  timestamp
>  etc...
> 
> that way you could sort installed packages by size:
> 
>   apk info --format "%I %P" '*' | sort -n

All of these are welcome ideas, from my side.


On Fri, Jun 28, 2013 at 04:03:54PM +0200, Natanael Copa wrote:
> On Fri, 28 Jun 2013 13:35:31 +0300
> Timo Teras <timo.teras@iki.fi> wrote:
> 
> > > diff --git a/src/apk.c b/src/apk.c
> > > index c3709e7..4bac0ba 100644
> > > --- a/src/apk.c
> > > +++ b/src/apk.c
> > > @@ -46,6 +46,7 @@ static struct apk_option generic_options[] = {
> > >  				required_argument, "DIR" },
> > >  	{ 'X', "repository",	"Use packages from REPO",
> > >  				required_argument, "REPO" },
> > > +	{ 0x113, "no-log",      "Don't write to log" },
> > >  	{ 0x101, "progress",	"Show a progress bar" },
> > >  	{ 0x10f, "progress-fd",	"Write progress to fd",
> > 
> > This might have non-wanted implications on the tmpfs initramfs
> > bootstrap.
> > 
> > I've been thinking that we need some flag for apk to indicate that it's
> > the tmpfs initial initramfs call to get proper defaults for flags like
> > these.
> 
> I am generally against apk trying to be smart but one option is to
> check if --root is specified and if destination directory is a tmpfs.
> 
> maybe add a --bootstrap option which would correspond to:
>  --overlay-from-stdin
>  --no-network
>  --clean-protected
>  --initdb
> 
> I suppose the challenge here is that many of the options are
> conditional. For example, if there are no apkovl, there should be no
> --overlay-from-stdin (an be worked around with /dev/null tricks) and if
> there are PXE network repos we replace the --no-network with
> --update-cache.
> 
> The --clean-protected is currently also optional with the boot option "keep_apk_new"

No comment here from me, I just wanted to extract and propagate this
part of the discussion.


-- 
Dubiousjim
dubiousjim@gmail.com


---
Unsubscribe:  alpine-devel+unsubscribe@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---