I proudly present: first version of apt-release
Hello Paul,
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.
> What would actually be the best way to work on this? I would suggest I
> get access to the git repository and work on a feature branch (if you
> trust me with that).
I do trust you with that (or also landing smaller fixes in master). Curiously,
I'm only a member of https://alioth.debian.org/projects/autopkgtest/, not an
admin, so Ian or Zack need to add you (and perhaps turn me into an admin).
Until then, you can also push the tree to github or similar, where it's nice to
comment/discuss inline.
> --- 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).
> --- 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?
> self.sp = None
> self.lastsend = None
> self.scratch = None
> @@ -63,6 +63,7 @@ class Testbed:
> self.setup_commands = setup_commands
> self.setup_commands_boot = setup_commands_boot
> self.add_apt_pockets = add_apt_pockets
> + self.add_apt_releases = add_apt_releases
> self.copy_files = copy_files
> self.initial_kernel_version = None
> # tests might install a different kernel; [(testname, reboot_marker, kver)]
> @@ -71,10 +72,11 @@ class Testbed:
> self.last_test_name = ''
> self.last_reboot_marker = ''
> self.eatmydata_prefix = []
> - self.apt_pin_for_pockets = []
> + self.apt_pin_for_releases = []
Well done for unifying this, as this is being used for re-trying without
pinning in case of uninstallability, which isn't specific to series vs. pocket.
> self.nproc = None
> self.cpu_model = None
> self.cpu_flags = None
> + self.default_release = None
>
> try:
> self.devnull = subprocess.DEVNULL
> @@ -206,7 +208,7 @@ class Testbed:
> def _opened(self, pl):
> self.scratch = pl[0]
> self.deps_installed = []
> - self.apt_pin_for_pockets = []
> + self.apt_pin_for_releases = []
> self.recommends_installed = False
> self.exec_cmd = list(map(urllib.parse.unquote, self.command('print-execute-command', (), 1)[0].split(',')))
> self.caps = self.command('capabilities', (), None)
> @@ -261,7 +263,7 @@ class Testbed:
> def run_setup_commands(self):
> '''Run --setup-commmands and --copy'''
>
> - if not self.setup_commands and not self.add_apt_pockets and not self.copy_files:
> + if not self.setup_commands and not self.add_apt_pockets and not self.add_apt_releases and not self.copy_files:
> return
>
> adtlog.info('@@@@@@@@@@@@@@@@@@@@ test bed setup')
> @@ -271,18 +273,27 @@ class Testbed:
>
> # create apt sources for --apt-pocket
> for pocket in self.add_apt_pockets:
> - pocket = pocket.split('=', 1)[0] # strip off package list
> - script = '''sed -rn 's/^(deb|deb-src) +(\[.*\] *)?([^ ]*(ubuntu.com|debian.org|ftpmaster|file:\/\/\/tmp\/testarchive)[^ ]*) +([^ -]+) +(.*)$/\\1 \\2\\3 \\5-%s \\6/p' /etc/apt/sources.list `ls /etc/apt/sources.list.d/*.list 2>/dev/null|| true` > /etc/apt/sources.list.d/%s.list; for retry in 1 2 3; do apt-get --no-list-cleanup -o Dir::Etc::sourcelist=/etc/apt/sources.list.d/%s.list -o Dir::Etc::sourceparts=/dev/null update 2>&1 && break || sleep 15; done''' % (pocket, pocket, pocket)
> + self.add_apt_releases.append(self._get_default_release_info() + '-' + pocket)
> +
> + # 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'.
> +
> + 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, and the comment is confusing; please rather
say "release name which occurs first in apt sources". And, in this case it's
reall a release :-)
> 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?
> index bff73c0..0790c6f 100755
> --- a/tests/adt-run
> +++ b/tests/adt-run
Likewise, this shouldn't be touched any more. This is only to ensure that the
backwards compat stuff doesn't regress. It's a lot of code duplication, but it
will go away after stretch.
> 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=?
> + def test_apt_release(self):
> + '''--apt-release'''
> +
> + p = self.build_src('Tests: pass\nDepends:\nRestrictions: needs-root\n',
> + {'pass': '#!/bin/sh -e\ntest -e /etc/apt/sources.list.d/unstable.list'})
> +
> + apt_dir = os.path.join(self.chroot, 'etc', 'apt')
> + with open(os.path.join(apt_dir, 'sources.list'), 'w') as f:
> + f.write('''# comment
> +deb http://foo.ubuntu.com/ fluffy-updates main non-free
> +deb-src http://foo.ubuntu.com/ fluffy-updates main non-free
> +deb http://foo.ubuntu.com/ fluffy main non-free
> +deb-src http://foo.ubuntu.com/ fluffy main non-free
> +deb http://bar.debian.org/ fluffy extras
> +deb-src http://bar.debian.org/ fluffy extras
> +# third-party repo
> +deb http://something.else.net/ fluffy addons
> +# options
> +deb [trusted=yes arch=6510] http://foo.ubuntu.com/ fluffy main 6510
> +''')
> + with open(os.path.join(apt_dir, 'sources.list.d', 'restricted.list'), 'w') as f:
> + f.write('deb http://foo.ubuntu.com/ fluffy restricted\n')
> +
> + with open(os.path.join(apt_dir, 'sources.list.d', 'non-default-release.list'), 'w') as f:
> + f.write('deb http://lib.ubuntu.com/ grumpy main\n')
> +
> + (code, out, err) = self.runtest(['-B', '-d', p, '--apt-release', 'unstable'])
> +
> + # test should succeed
> + self.assertEqual(code, 0, err)
> + self.assertRegex(out, 'pass\s+PASS', out)
> +
> + # verify proposed.list
> + with open(os.path.join(apt_dir, 'sources.list.d', 'unstable.list')) as f:
> + self.assertEqual(f.read(), '''deb http://foo.ubuntu.com/ unstable main non-free
> +deb-src http://foo.ubuntu.com/ unstable main non-free
> +deb http://bar.debian.org/ unstable extras
> +deb-src http://bar.debian.org/ unstable extras
> +deb [trusted=yes arch=6510] http://foo.ubuntu.com/ unstable main 6510
> +deb http://lib.ubuntu.com/ unstable main
> +deb http://foo.ubuntu.com/ unstable restricted
> +''')
Oh, that's interesting -- I hadn't actually expected that you really want to
add all existing?sources and rewrite them for "unstable". 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.
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?
Thanks!
Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/autopkgtest-devel/attachments/20170126/86ad7df6/attachment.sig>
Reply to: