I proudly present: first version of apt-release

[Paul Gevers]

Hi,

On 26-01-17 13:50, Martin Pitt wrote:
> Paul Gevers [2017-01-23 22:19 +0100]:
>> It looks like I have something working that generalizes --apt-pocket in
>> an --apt-release option and use that generalization to still support
>> that --apt-pocket. While at it, I have added a check on the suite name
>> versus code name issue I discussed earlier.
> 
> Nice! Comments below inline.

Appreciated, thanks.

> Until then, you can also push the tree to github or similar, where it's nice to
> comment/discuss inline.

I'll probably do that shortly then.

>> --- a/lib/adt_run_args.py
>> +++ b/lib/adt_run_args.py
> 
> TBH, I wouldn't bother with adding new options to adt-run. It's been deprecated
> for a while and I really don't want new deployments to use adt-run (I want to
> drop it after the stretch release).

I thought about that, so ack. Will remove it again.

>> --- a/lib/adt_testbed.py
>> +++ b/lib/adt_testbed.py
>> @@ -46,7 +46,7 @@ timeouts = {'short': 100, 'copy': 300, 'install': 3000, 'test': 10000,
>>  class Testbed:
>>      def __init__(self, vserver_argv, output_dir, user,
>>                   setup_commands=[], setup_commands_boot=[], add_apt_pockets=[],
>> -                 copy_files=[]):
>> +                 add_apt_releases=[], copy_files=[]):
> 
> This is probably bikeshedding matter, but my feeling is that this new
> option/variables should be --apt-series/SERIES/add_apt_series. I. e. if you
> have "jessie-backports", then the release of that is "jessie", the pocket is
> "backports", and the series is the combination ("jessie-backports"), AFAIUI?

I think it is worth bikeshedding about, to be honest. I must admit that
to me it isn't 100% clear which name means what. I have the impression
it is used for different things in different contexts.

From $(man apt):
codename (jessie, stretch, sid ...)
suite name (stable, testing, unstable)

From $(man apt_preferences):
the Archive: or Suite: ... release a=stable
the Codename: ... release n=stretch

While looking the latter man page, I now believe that --apt-release is
indeed wrong. Both apt manpages aren't talking about series, so I don't
think that is the right name either.

>> +        # Make sure we have the original list of apt/sources.list.d available
>> +        apt_sources_org = self.check_exec(['sh', '-ec', 'ls /etc/apt/sources.list.d/*.list 2>/dev/null || true'], stdout=True).replace('''
>> +''', ' ')
> 
> "_org" sounds strange (like a domain name), can you please rename to
> apt_sources_list_d? Also, please replace that confusing ''' ''' line break
> with '\n'.

Automatism: in $DAYJOB we typically add that suffix for variables that
store the original value. I think it should be clear that new files
aren't added on purpose, is the comment above it enough? About the line
break, I wasn't aware that .replace() doesn't treat the first input
literally (and didn't spend the time for a ridiculous quick test, shame
on me).

>> +
>> +    def _get_default_release_info(self):
>> +        '''Determine which release (suite name) is considered apt's default'''
>> +        # Note: we don't check what APT really thinks of this,
>> +        # i.e. APT::Default-Release
> 
> The _info() suffix is superfluous,

Agree, originally that function was doing slightly more.

> and the comment is confusing; please rather
> say "release name which occurs first in apt sources". And, in this case it's
> really a release :-)

It does more than only determine the release name in apt source. It also
converts this to the archive / suite name, suitable for apts a= pinning.

>> diff --git a/lib/autopkgtest_args.py b/lib/autopkgtest_args.py
>> index 8f28443..bc2f31f 100644
>> --- a/lib/autopkgtest_args.py
>> +++ b/lib/autopkgtest_args.py
>> @@ -265,6 +265,13 @@ for details.'''
>>                           'If packages are given, set up apt pinning to use '
>>                           'only those packages from POCKETNAME; src:srcname '
>>                           ' expands to all binaries of srcname')
> 
> Can you please change the existing --apt-pocket documentation here to say
> 'from release-POCKETNAME;" to clarify the difference between a full series name
> and a pocket?

Sounds great. I looked at it briefly, but couldn't come up with short
text. Your proposal is even shorter than things I had in mind.

>> diff --git a/tests/autopkgtest b/tests/autopkgtest
>> index 6d63954..add5204 100755
>> --- a/tests/autopkgtest
>> +++ b/tests/autopkgtest
>> @@ -1502,6 +1502,7 @@ class ChrootRunner(AdtTestCase, DebTestsAll, DebTestsFailureModes):
>>                      f.write('if [ "$1" = source ]; then cp -r /aptget-src $4-1; fi\n')
>>                  if cmd == 'apt-cache':
>>                      f.write('if [ "$1" = showsrc ]; then printf "Package-List:\\n $3 deb utils optional arch=any\\nFormat: 1.0\\n"; fi\n')
>> +                    f.write('if [ "$1" = policy ] ; then printf "Package files:\\n 100 /var/lib/dpkg/status\\n     release a=now\\n 500 https://bar.debian.org/debian stitch/main amd64 Packages\\n     release o=Debian,a=fluffy,n=stitch,l=Debian,c=main,b=amd64\\n     origin bar.debian.org\\nPinned packages:\\n"; fi\n')
> 
> Please remind me, what's the difference between a= and n=?

Copied from above, (and explained in apt_preference man):
With a one needs to provide the Archive or Suite name, like unstable,
testing, stable, old-stable
With n one needs to provide the codename, like sid, stretch, jessie.

I believe (but haven't checked) that in Ubuntu those are the same.

> Oh, that's interesting -- I hadn't actually expected that you really want to
> add all existing⎵sources and rewrite them for "unstable".

I admit I have been struggling with what I wanted. I made the regression
test such that it is at least visible, so I am very glad that you noticed.

> This is certainly
> wrong for the former "lib.ubuntu.com grumpy" as that is a different series than
> the default "fluffy" one. It's debatable if this should be done for the
> third-party repo, as they commonly don't have other series either. One could
> argue either way, but this seems to be brittle in both directions.

Full ack on discussing. I believe there is no "proper" solution, so I
wanted to at least talk about this in documentation. I am even wondering
what people expect with this kind of setup and autopkgtesting with
automatic adding of stuff. I was hoping we could consider this corner
case unsupported, because I didn't come up with a (simple) solution so
far. We could only add the archives for cases where the release matches
(both suite and code name), but especially in shell that needs loads
more logic.

> FYI, Simon propopsed a more generic way to add an entire apt line in #851568,
> which then would resolve any ambiguity like the ones above. The charm of
> --apt-pocket is that it can stay a hardcoded value in a CI system, but the same
> won't really extend for full series names. So maybe for this having an
> --apt-source which gets the full line (maybe with some magic <mirror>
> replacement from the existing apt sources) might be both more flexible and less
> confusing?

I'll let that idea sink in. I think that would mean that we would treat
--apt-pocket and --apt-source quite different again, but maybe that
isn't too bad.

Paul

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.alioth.debian.org/pipermail/autopkgtest-devel/attachments/20170126/55652746/attachment.sig>