Bug#1059296: hamster-time-tracker: CVE-2023-36250

2023-12-24 Thread Matthijs Kooijman
forwarded 1059296 https://github.com/projecthamster/hamster/issues/750
thanks

Hi Moritz,

Thanks for bringing this to attention, this was not reported upstream
yet.

> https://github.com/BrunoTeixeira1996/CVE-2023-36250/blob/main/README.md
> sounds a little bogus, I don't see how this crosses any security boundary?

AFAICS it does not cross any boundary, but if it allows arbitrary
commands to be executed when just importing a CSV file, that would still
be unexpected and a security issue.

I haven't looked at the code yet, but hope to do so in the common days.
Let's keep further discussion about this upstream for now.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#1053950: openttd: diff for NMU version 13.4-0.1

2023-11-26 Thread Matthijs Kooijman
Hi Maytham,

> Nice! It's not on the Salsa repo, so I'm assuming you're yet to push your
> changes.
Yup. I've justed pushed and uploaded!

> Thanks for your quick responses, and thanks for maintaining this awesome
> package! Looking forward to the new version. :)
Thanks for your interest and effort as well!

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#1053950: openttd: diff for NMU version 13.4-0.1

2023-11-26 Thread Matthijs Kooijman
Hi Maytham,

> nmudiff (from devscripts) did that gigantic diff, but I've followed
> your workflow in my fork of the repo at [1], if you want to just copy
> the commits over (and change the Debian revision from -0.1 to -1).
Ah, I already imported the tarball myself :-)

> I've built it on my computer, and have ran it through the usual tests,
> and the only issue is that Lintian says the override for the missing
> man page is unnecessary.

Yeah, that's because on build, lintian tests both the openttd and
openttd-data package so it sees the manpage and thinks the override is
unneeded, but it *is* actually needed when testing the packages in
isolation.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#1053950: openttd: diff for NMU version 13.4-0.1

2023-11-25 Thread Matthijs Kooijman
Hi Maytham,

> I've prepared an NMU for openttd (versioned as 13.4-0.1). The diff
> is attached to this message.
Thanks for your patch. I should have uploaded a new OpenTTD version
a long time ago, so let met correct that by handling the update now.
I'll go through my normal git-based workflow rather than using your NMU
though, AFAICS you've made no changes to the packaging except import the
upstream tarball, right?

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#1032857: please consider adding the jgrpp patches

2023-04-30 Thread Matthijs Kooijman
Hey Toni,

> please consider packaging the jgrpp patches, too.

Thanks for your suggestion. What did you have in mind exactly?

I won't include third-party patches in the main build, since that
affects multiplayer compatibility (i.e. a patched build will be
incompatible with other people playing unpatched builds).

I could consider including a non-patched and a patched build
side-to-side (i.e.  in two different binary packages), but I'm not sure
I want to go down this path either (this is just one set of patches, but
I'm pretty sure there's others and before we know it we'll be packaging
a dozen of different patch sets...).

IMHO the right path to making these patches more accessible, is to
submit them for inclusion upstream (possibly after improving the patch
quality).

So I'm inclined to close this as wontfix.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#1027860: hamster-time-tracker: cannot start because module 'gettext' has no attribute 'bind_textdomain_codeset'

2023-01-04 Thread Matthijs Kooijman
forwarded 1027860 https://github.com/projecthamster/hamster/issues/710
thanks

Hi,

> AttributeError: module 'gettext' has no attribute 'bind_textdomain_codeset'

Thanks for reporting this. There is also an upstream issue about this,
with a suggested workaround:
https://github.com/projecthamster/hamster/issues/710

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#793749: RFP: telegraf -- plugin-driven server agent for reporting metrics into InfluxDB

2022-10-17 Thread Matthijs Kooijman
Package: wnpp
Followup-For: Bug #793749

FYI: It seems that Ubuntu has started packaging telegraf in 2020:

https://packages.ubuntu.com/source/kinetic/telegraf
http://changelogs.ubuntu.com/changelogs/pool/universe/t/telegraf/telegraf_1.22.3+ds1-0ubuntu1/changelog



Bug#1013356: fzf: Bash completions not active by default

2022-06-22 Thread Matthijs Kooijman
Package: fzf
Version: 0.30.0-1+b1
Severity: normal

Hi,

README.Debian says:

  Note, since fzf 0.29.0-1, the bash completion is installed for bash by
  default. Feel free to ignore the following instruction for fzf >=
  0.29.0-1.

It seems this means that the completion is installed as
/usr/share/bash-completion/completions/fzf

However, completions from that directory are not loaded by default, but
are loaded dynamically when the user tries to complete arguments to
their command.

See e.g.:

https://salsa.debian.org/debian/bash-completion/-/blob/master/bash_completion#L2175

In practice, this means that in a new shell, doing "cd **" does not
offer completion. If I then do "fzf ", the completion is loaded,
and after that "cd **" *does* offer fzf completion.

This was tested on Ubuntu 22.04, but with the sid version of fzf
(0.30.0-1+b1) and bash-completion (2:2.11-6), so I'm assuming the same
behavior happens on Debian.


I'm not sure if there is a way for the package to bypass this dynamic
loading and have a snippet be loaded automatically (other than putting
a file in `/etc/bash_completion.d`, but that seems to be for
compatibility only).

What does seem to work is to load it explicitly in `~/.bashrc`:

  source /usr/share/bash-completion/completions/fzf

So maybe that chould be documented?

Gr.

Matthijs

-- System Information:
Debian Release: bookworm/sid
  APT prefers jammy-updates
  APT policy: (500, 'jammy-updates'), (500, 'jammy-security'), (500, 'jammy'), 
(100, 'jammy-backports'), (50, 'unstable-debug'), (50, 'testing-debug'), (50, 
'stable-security'), (50, 'stable-debug'), (50, 'unstable'), (50, 'testing'), 
(50, 'stable')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 5.15.0-25-generic (SMP w/8 CPU threads)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

-- no debconf information



Bug#1003269: openttd: typo in suggested timidity package

2022-01-07 Thread Matthijs Kooijman
Hey Daniel,

> timidity was recommended but now timdity is suggested instead
Bugger.

I've fixed this locally, will be included in the next upload (but I'm
not going to reupload just for this, I think).

Thanks for reporting!

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#1002952: build-rdeps: Inconsistent defaults on Ubuntu (or other non-Debian systems)

2022-01-01 Thread Matthijs Kooijman
Package: devscripts
Version: 2.21.7
Severity: normal

Hi,

I was trying to use build-rdeps on Ubuntu today, and the default settings seem
unusable there. I tried both the Ubuntu version from impish and the Sid
version, both with the same result:

  $ sudo build-rdeps -d dh-cmake
  DEBUG: Package => dh-cmake
  DEBUG: running with dose-ceve resolver
  DEBUG: buildarch=amd64 hostarch=amd64
  DEBUG: Running 'apt-get' 'indextargets' 'DefaultEnabled: yes' 'Origin:
  Debian'
  build-rdeps: unable to find sources files.
  Did you forget to run apt-get update (or add --update to this command)?
  at /usr/bin/build-rdeps line 520.

Looking at the source code, I see that there does seem to be some code
in place to handle non-Debian vendors as well, but I think this does not
work well currently.

By default, the script only looks at sources that have 'Origin: Debian',
regardless of what the current system is:

  
https://salsa.debian.org/debian/devscripts/-/blob/master/scripts/build-rdeps.pl#L157
  my $opt_origin = 'Debian';

However, it then further limits sources to "development" releases only:

  
https://salsa.debian.org/debian/devscripts/-/blob/master/scripts/build-rdeps.pl#L227
  sub is_devel_release {
  my $ctrl = shift;
  if (get_current_vendor() eq 'Debian') {
  return $ctrl->{Suite} eq 'unstable' || $ctrl->{Codename} eq 'sid';
  } else {
  return $ctrl->{Suite} eq 'devel';
  }
  }

In this case, it *does* use the current vendor to determine the default
release to check for, meaning running build-rdeps without additional
options on non-Debian systems will never work (not even if you have
Debian deb-src lines in your sources.list as I have).

It seems like adding an explicit --origin Ubuntu would help here,
except:
 - Using a "devel" release of Ubuntu is apparently not a common practice
   (anymore):
   https://bugs.launchpad.net/ubuntu/+source/apt/+bug/1728616/comments/2
 - If you *do* use e.g. `deb-src http://archive.ubuntu.com/ubuntu/ devel main`,
   this produces "Release: devel", not "Suite: devel", so the current
   code still does not match.

More generally, I wonder if build-rdeps should really limit itself to a
devel release only. This poses an additional requirement on your
sources.list (you need to have deb-src for sid or devel), which is not
documented in the manpage and not hinted at by the error message.

I can imagine it would *favor* unstable/devel if available (or maybe
testing if unstable is not found) , but fall back to the most recent
release (based on the Version field), or failing that, *any* available
src release instead?


As for the conflicting defaults, I can imagine:
 - Adding a --vendor option, defaulting to Dpkg::Vendor::get_current_vendor()
 - Pick the Origin based on the vendor (and maybe apply no origin limit
   for unknown vendors)
 - Pick the default distribution based on the vendor, like now (but
   checking against Release: devel instead of Suite:devel

Alternatively, if the "priority"-based scheme suggested above is
implemented, maybe there is no need at all to switch the distribution
based on the vendor at all. If you just apply a priority to each release
(sid/unstable/devel get max priority, testing nearly max priority,
others based on Version) this works regardless of the current vendor,
and you can switch between vendors based on the Origin (which should
probably still switch its default based on the vendor).


Note that there is also the matter of the list of components being
hardcoded currently, but that is already the subject of #913551.

As for making the current version work, what I can now do is:
 - Add a Ubuntu "devel" release to sources.list, patch build-rdeps to
   check Release: devel instead of Suite: devel, and run with `--origin
   Ubuntu".
 - Run with "--origin Ubuntu --distribution impish" (or whatever distro
   you're using).
 - Add "sid" sources to sources.list, and run with `--distribution sid`.

Gr.

Matthijs

-- Package-specific info:

--- /etc/devscripts.conf ---
Empty.

--- ~/.devscripts ---
Not present

-- System Information:
Debian Release: 11.0
  APT prefers jammy
  APT policy: (500, 'jammy'), (500, 'impish-updates'), (500, 
'impish-security'), (500, 'impish'), (100, 'impish-backports'), (50, 
'unstable-debug'), (50, 'testing-debug'), (50, 'stable-security'), (50, 
'stable-debug'), (50, 'unstable'), (50, 'testing'), (50, 'stable')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 5.13.0-22-generic (SMP w/4 CPU threads)
Kernel taint flags: TAINT_WARN
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages devscripts depends on:
ii  dpkg-dev  1.20.9ubuntu2
ii  fakeroot  1.25.3-1.1ubuntu2
ii  file  1:5.39-3
ii  gnupg 2.2.20-1ubuntu4
ii  gnupg22.2.20-1ubuntu4
ii  gpgv 

Bug#1002944: build-rdeps: Shows unfriendly errors when deb-src lines are missing

2022-01-01 Thread Matthijs Kooijman
Package: devscripts
Version: 2.21.7
Severity: normal

Hi,

when running build-rdeps without deb-src lines in sources.list, it
produces perl errors, rather than providing useful feedback. This also
happens when a deb-src line is present for the main packages, but
missing for debug packages.

E.g. with these sources:

  deb http://ftp.nl.debian.org/debian/ sid main non-free contrib
  deb http://deb.debian.org/debian-debug/ unstable-debug main

I get:

  $ sudo build-rdeps -d dh-cmake --distribution sid
  DEBUG: Package => dh-cmake
  DEBUG: running with dose-ceve resolver
  DEBUG: buildarch=amd64 hostarch=amd64
  DEBUG: Running 'apt-get' 'indextargets' 'DefaultEnabled: yes' 'Origin: Debian'
  Reverse Build-depends in main:
  --

  Use of uninitialized value $source_file in concatenation (.) or string at 
/usr/bin/build-rdeps line 336.
  DEBUG: executing: dose-ceve -T debsrc -r dh-cmake -G pkg 
--deb-native-arch=amd64 
deb:///var/lib/apt/lists/deb.debian.org_debian-debug_dists_unstable-debug_main_binary-amd64_Packages
 debsrc://Fatal error in module dose_common.input: 
   Input file  does not exist.
  No reverse build-depends found for dh-cmake.

  Reverse Build-depends in main:
  --

  Use of uninitialized value $source_file in concatenation (.) or string at 
/usr/bin/build-rdeps line 336.
  DEBUG: executing: dose-ceve -T debsrc -r dh-cmake -G pkg 
--deb-native-arch=amd64 
deb:///var/lib/apt/lists/ftp.nl.debian.org_debian_dists_sid_main_binary-amd64_Packages
 debsrc://Fatal error in module dose_common.input: 
   Input file  does not exist.
  No reverse build-depends found for dh-cmake.

  Reverse Build-depends in contrib:
  --

  Use of uninitialized value $source_file in concatenation (.) or string at 
/usr/bin/build-rdeps line 336.
  DEBUG: executing: dose-ceve -T debsrc -r dh-cmake -G pkg 
--deb-native-arch=amd64 
deb:///var/lib/apt/lists/ftp.nl.debian.org_debian_dists_sid_contrib_binary-amd64_Packages
 debsrc://Fatal error in module dose_common.input: 
   Input file  does not exist.
  No reverse build-depends found for dh-cmake.

  Reverse Build-depends in non-free:
  --

  Use of uninitialized value $source_file in concatenation (.) or string at 
/usr/bin/build-rdeps line 336.
  DEBUG: executing: dose-ceve -T debsrc -r dh-cmake -G pkg 
--deb-native-arch=amd64 
deb:///var/lib/apt/lists/ftp.nl.debian.org_debian_dists_sid_non-free_binary-amd64_Packages
 debsrc://Fatal error in module dose_common.input: 
   Input file  does not exist.
  No reverse build-depends found for dh-cmake.


Looking at the source, it seems that collectfiles() looks at Sources files, but
also Packages to find the arch. So for dists without a deb-src line, this lets
collectfiles() return entries that have an `Architecture` field, but no
`sources` field,

This is probably easy to fix by just letting findreversebuilddeps() check for
missing `sources` and showing an appropriate message.

Gr.

Matthijs


-- Package-specific info:

--- /etc/devscripts.conf ---
Empty.

--- ~/.devscripts ---
Not present

-- System Information:
Debian Release: 11.0
  APT prefers impish-updates
  APT policy: (500, 'impish-updates'), (500, 'impish-security'), (500, 
'impish'), (100, 'impish-backports'), (50, 'testing-debug'), (50, 
'stable-security'), (50, 'stable-debug'), (50, 'unstable'), (50, 'testing'), 
(50, 'stable')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 5.13.0-22-generic (SMP w/4 CPU threads)
Kernel taint flags: TAINT_WARN
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages devscripts depends on:
ii  dpkg-dev  1.20.9ubuntu2
ii  fakeroot  1.25.3-1.1ubuntu2
ii  file  1:5.39-3
ii  gnupg 2.2.20-1ubuntu4
ii  gnupg22.2.20-1ubuntu4
ii  gpgv  2.2.20-1ubuntu4
ii  libc6 2.34-0ubuntu3
ii  libfile-dirlist-perl  0.05-2
ii  libfile-homedir-perl  1.006-1
ii  libfile-touch-perl0.11-1
ii  libfile-which-perl1.23-1
ii  libipc-run-perl   20200505.0-1
ii  libmoo-perl   2.004004-1
ii  libwww-perl   6.53-1
ii  patchutils0.4.2-1
ii  perl  5.32.1-3ubuntu3
ii  python3   3.9.4-1build1
ii  sensible-utils0.0.14
ii  wdiff 1.2.2-2build2

Versions of packages devscripts recommends:
ii  apt 2.3.9
ii  curl7.74.0-1.3ubuntu2
ii  dctrl-tools 2.24-3
ii  debian-keyring  2021.09.25
ii  dput1.1.0ubuntu2
ii  dupload 2.9.6
ii  equivs  2.3.1
ii  libdistro-info-perl 1.0
ii  libdpkg-perl1.20.9ubuntu2
ii  libencode-locale-perl   1.05-1.1
ii  

Bug#1002913: openttd: FBTFS

2021-12-31 Thread Matthijs Kooijman
Hi Aurelien,

Thanks for reporting, I had already seen the failure and am working on
a fix, probably next weekend. The issue is caused by the buildds
building arch and arch-indep separately, which exposes a problem in the
rules file, but I hadn't tested this scenario before upload.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#984158: grfcodec: diff for NMU version 6.0.6-5.1

2021-12-20 Thread Matthijs Kooijman
Hi Adrian,

It took a while longer than anticipated, because of a problem with last
month's keyring update, but that has now been resolved, so my uploads
have come through.

> packages without valid signature are usually silently dropped,
> expect that you might have to reupload.
Maybe packages with an expired signature are handled differently, since
my packages were processed without reupload now.

Thanks again for your interest and time!

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#984158: grfcodec: diff for NMU version 6.0.6-5.1

2021-11-23 Thread Matthijs Kooijman
Hi Adrian,

> I've prepared an NMU for grfcodec (versioned as 6.0.6-5.1) and uploaded 
> it to DELAYED/15. Please feel free to tell me if I should cancel it, or
> to use the changes for a maintainer upload instead.
Thanks for your interest and picking this up!

However, I had actually *just* uploaded a new version myself yesterday, but
it seems they are not picked up. I suspect this is because my gpg key
expired and the validity extension I did a while ago isn't picked up by
the keyring yet. As soon as it is, I expect my pending packages will be
processed.

See also https://salsa.debian.org/openttd-team/grfcodec/-/commits/master/

I suspect it would be best to cancel your upload, and use my version
instead.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#984158: grfcodec: ftbfs with GCC-11: Fixed upstream

2021-04-24 Thread Matthijs Kooijman
forwarded 984158 https://github.com/OpenTTD/grfcodec/issues/12
tags 984158 fixed-upstream
thanks

Hi Matthias,

thanks for reporting this issue. I've forwarded it upstream and they
have fixed it, should be included in their next release (6.0.7 or
6.1.0).

Gr

Matthijs


signature.asc
Description: PGP signature


Bug#780280: dak: generate rejection mail for mails with expired signature

2021-02-26 Thread Matthijs Kooijman
Hey folks,

this issue still seems to exist, I just discovered that an upload I did
three months ago was never processed because I forgot to push my
extended key to Debian, which is a bit of a bummer.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#980641: nml: builds with patch

2021-02-14 Thread Matthijs Kooijman
Hi Phil,

> Indeed, I've cleaned up my local test and pushed to salsa:
>
> https://salsa.debian.org/emorrp1/nml/-/commit/27c0aea7cd2670462c24246caf510d7dd8cb99dd
Thanks! I think to be really correct, though, I'd have to backup the old
files and restore them on clean (or maybe make a copy of the entire
regression directory and run tests in there).

> > I'll see if upstream maybe wants to do a release with these changes
> > included, might be the easiest route...
>
> With 84 commits to master, I'm not convinced that would qualify for an
> unblock.
Hm, I hadn't really thought about the freeze. Seems we're still in soft
freeze, and this not a very high-profile package, so I don't think a
manual unblock is needed yet, but looking at the git log, it seems
it's not just a few small fixes, also new features and refactors that
might not be appropriate in this stage, so maybe better to backport the
fix indeed.

I also just noticed that I missed the nml 0.5.3 release in september.
Given that's just a few changes, mostly for compatibility, I'm inclined
to still include that release in my next upload, even with the freeze.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#980641: nml: builds with patch

2021-02-13 Thread Matthijs Kooijman
Hi Phil, Mathias,

Thanks for your interest in nml and your effort in getting this bug
squashed :-)

> It would be a shame if openttd got autorm'ed just as the bullseye
> freeze starts in earnest.

Yeah, I'll make sure that doesn't happen.

> Upstream have re-exported the pcx files and I can confirm nml now builds
> correctly with these 3 files copied into place before tests.
Cool, thanks for confirming. It would be obvious to just backport these
changes, but I think the quilt patches used by the Debian patches cannot
represent changes to binary files, so it would be a bit more hassle
(probably needs some scripting in debian/rules) to include these
changes.

I'll see if upstream maybe wants to do a release with these changes
included, might be the easiest route...

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#975528: libinput-tools: debug-gui command not available

2020-11-23 Thread Matthijs Kooijman
Package: libinput-tools
Version: 1.15.5-1ubuntu0.1
Severity: normal

Hi,

the manpage, `libinput --help` output and upstream [1] document a
`libinput debug-gui` command, but that does not seem to be available:

  $ libinput debug-gui
  libinput: debug-gui is not a libinput command or not installed. See 'libinput 
--help'

  $ libinput  --help |grep -A 1 gui
  debug-gui
  Display a simple GUI to visualize libinput's events.

I've tested this on Ubuntu, but looking at the `libinput-tools` file
list [2] for 1.16.3-1 from unstable, it seems that `libinput`
subcommands are shipped as separate binaries, but `debug-gui` is indeed
not included, so I'm assumning this is not Ubuntu-specific.

[1]: 
https://wayland.freedesktop.org/libinput/doc/latest/tools.html#libinput-debug-gui
[2]: https://packages.debian.org/sid/amd64/libinput-tools/filelist

Gr.

Matthijs

-- System Information:
Debian Release: bullseye/sid
  APT prefers focal-updates
  APT policy: (990, 'focal-updates'), (990, 'focal-security'), (990, 
'focal-backports'), (990, 'focal'), (50, 'unstable'), (50, 'testing'), (50, 
'stable'), (50, 'oldstable')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 5.4.0-52-generic (SMP w/4 CPU cores)
Kernel taint flags: TAINT_WARN
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), 
LANGUAGE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages libinput-tools depends on:
ii  libc6 2.31-0ubuntu9.1
ii  libevdev2 1.9.0+dfsg-1
ii  libinput101.15.5-1ubuntu0.1
ii  libudev1  245.4-4ubuntu3.3
ii  python3   3.8.2-0ubuntu2
ii  python3-libevdev  0.5-1
ii  python3-pyudev0.21.0-3ubuntu1

libinput-tools recommends no packages.

libinput-tools suggests no packages.

-- no debconf information



Bug#905866: uscan: prefers watch files from a random dir over debian/watch

2020-09-05 Thread Matthijs Kooijman
Hey Mattia,

> Thank you for this, I've now merged it.
Thanks!

> Also, I personally hate changing long-standing defaults.
Yeah, I can see that.

> > Maybe the default could be changed to only scan the current directory
> > *if* it is a debian source tree, and default to recursive scanning if
> > not? That would support both the "Run on a single package" and "Run on a
> > collection of packages" usecases neatly?
> 
> That's too surprising.  Changing behaviour that way just due to the
> surrounding files is too unexpected for me.
It might end up doing the right thing in most cases, but I can see it
could also be surprising, which is indeed a downside of this approach.

> Regardless, I'd accept an MR that would implement:
Ok, sounds good. I'd like to submit such a MR, but it's highly likely
that I'll not find the time in the near future, or maybe not at all, so
if anyone else wants to do this, feel free.

> Also perhaps add a relevant config item that would switch the default
> locally, so that one can set, e.g., USCAN_RECURSIVE=no in their
> ~/.devscripts and have it re-enabled with --recursive as needed.
That also sounds like a good idea, that at least allows people to change
their own defaults.

> > One open question is what constitutes a "source tree" exactly for the
> > purpose of the default operation.
> 
> [ -f debian/changelog -a -f debian/watch ] should do IMHO.  Without
> parsing, it would just fail a few moments later anyway.
Given you do not want to change the default as I suggested, I think this
question is not actually relevant anymore.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#926455: mail_autoremovals: incorrect version number in email warning

2020-08-17 Thread Matthijs Kooijman
Package: release.debian.org
Followup-For: Bug #926455

Hi,

I also ran into this today. To me, it seems the observable problem is
not so much an incorrect version number in the email warning, but the
fact that the email warning is sent out at pretty much exactly the
moment the autorm for a package is *cleared*, which makes it a very
confusing email. The version number in there is also wrong, or at least
contains the version that fixed the bug that caused autorm, indeed.

I noticed this for the grfcodec package:

  Date: Mon, 17 Aug 2020 04:39:06 +
  From: Debian testing watch 
  To: grfco...@packages.debian.org
  Subject: grfcodec 6.0.6-5 MIGRATED to testing

  FYI: The status of the grfcodec source package
  in Debian's testing distribution has changed.

Previous version: 6.0.6-4
Current version:  6.0.6-5


  Date: Mon, 17 Aug 2020 04:39:05 +
  Subject: grfcodec is marked for autoremoval from testing

  grfcodec 6.0.6-5 is marked for autoremoval from testing on 2020-09-11

  It is affected by these RC bugs:
  957307: grfcodec: ftbfs with GCC-10
   https://bugs.debian.org/957307


Looking at the mailer script [1], it seems it tracks the most recent autorm
email notification timestamp (to make sure to send out a notification only
every 20 days) for each package version separately. So this makes me suspect
that when a package migrates to testing that is subject to autorm:

 1 the new version is first inserted into the `testing_autoremovals` table
 2 the mail_autoremovals.pl script runs, sees this new version for which no
   notification was sent before, so immediately sends out a mail notification
 3 the autorm status is cleared for the package, because the fixing version was
   migrated to testing

I'm not quite sure where all this is orchestrated, so I couldn't check this in
any code (other than the mail_autoremovals.pl code). Also, I can't remember
seeing this behaviour before for packages where autorm was cleared by an
upload, so I suspect that there might be a race condition between two processes
here.

Regardless, it seems that the new, fixing, version should *never* end up in the
`testing_autoremovals` table, since that version is really never subject to
autorm AFAICS. So if my analysis is correct, maybe that's something that can be
fixed?

Gr.

Matthijs

[1]: 
https://salsa.debian.org/release-team/release-tools/-/blob/master/mailer/mail_autoremovals.pl



Bug#967933: ant-optional: Missing ant-junitlauncher.jar

2020-08-05 Thread Matthijs Kooijman
Package: ant-optional
Version: 1.10.6-1ubuntu0.1
Severity: normal

Hi,

I'm trying to run junit-based tests for the JOSM application using ant,
which does not work out of the box. The error I'm getting is:

  build.xml:491: The following error occurred while executing this line:
  build.xml:442: Problem: failed to create task or type junitlauncher
  Cause: the class 
org.apache.tools.ant.taskdefs.optional.junitlauncher.confined.JUnitLauncherTask 
was not found.

Which suggests the 
org.apache.tools.ant.taskdefs.optional.junitlauncher.confined package is
missing, which was added in ant 1.10.6 [1].

I'm not entirely familiar with this ecosystem, but I *think* this
package is supplied by upstream (the java sources are present in the
Debian source package) and should result in an ant-junitlauncher.jar
file, which I think should be shipped in the ant-optional Debian
package. However, that jar is not contained in any Debian package
AFAICS.

It seems there is an explicit whitelist of jar/pom files to install, and
this new jar was probably forgotten when it was added upstream?

Note that I tested on a slightly older Ubuntu version, but it seems the
newest Debian version has the same problem.

If my analysis is correct, could you make the changes so
ant-junitlauncher is included?

Gr.

Matthijs

[1]: 
https://ant.apache.org/manual/api/org/apache/tools/ant/taskdefs/optional/junitlauncher/confined/package-summary.html



Bug#875733: same with buster

2020-05-27 Thread Matthijs Kooijman
> Has anybody succeeded in running systemd inside an LXC container with
> "lxc.cap.drop = sys_admin" ?
Yup, on a Buster system, I'm using this config, which works:

https://github.com/daenney/Tika/blob/tika-host/etc/lxc/login/config

Not sure what the essential part is, but maybe you can compare this with
your own config and make it work from there.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#960419: dh_link: Add option to check for identical file contents before replacing

2020-05-15 Thread Matthijs Kooijman
Hey Niels,

> As far as I can tell, the underlying desire is to do some form of
> deduplication.
Yes, indeed.

> If so, then I think this is similar to #888397 with an expanded scope
> and a proposal for doing this via dh_link (rather than dh_installdocs
> or a new helper).

I guess my proposal is actually more narrow and specific. That other
report asks about more generic deduplication (run on the entire package,
or maybe a subset of files) that automatically finds duplicate files. I
guess that could be useful, but is also quickly a lot more complicated
(more work to find duplicates, which is the canonical one, any
exceptions). I'm also less inclined to completely automate this, I'd
rather make some more explicit choices (though something like "link files
in *this* directory to *that* directory if you find duplicates" could be
a nice compromise between automatic and manual work, maybe).

Regardless, my suggestion would allow manual and explicit deduplication
to become a bit easier, at the expense of having to manually track the
list of duplicate files on upstream changes (but with my suggestions,
detecting files that are no longer duplicated is automatic, and lintian
can I think already detect *new* duplicate files, so together that would
allow adequately fixing all duplicates).

I think that both issues are thus sufficiently separate (in how they
work and would be implemented) and can be valuable to eventually
implement side-by-side, so I would suggest keeping both issues open for
now.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#960540: git-buildpackage: Documentation unclear about pristine-tar when upstream source comes from git

2020-05-13 Thread Matthijs Kooijman
Package: git-buildpackage
Version: 0.9.15
Severity: normal

Hi Guido,

I'm trying to figure out whether I need pristine-tar or not, and I can't
quite figure it out based on the documentation.

I understand that, when upstream supplies a tarball, you can use
`gbp import-orig --pristine-tar` to import the tarball along with
pristine-tar data, and then `gbp buildpackage --pristine-tar` will
recreate the pristine tarball again (if needed).

However, what if there is no upstream tarball and sources are
merged using git directly? Is having a pristine tarball still relevant?
I can imagine that when multiple people check out the packaging repo and
build a package, they want to have the exact same tarball, so it would
make sense to:
 - When a tarball is first created, commit pristine-tar data for it.
 - When a tarball is created later, use the pristine-tar info to
   recreate an identical tarball.

Or does the `git archive` used to create tarballs from git already
produce consistent tarballs (since there is no upstream tarball to
match, this would be sufficient)? I could not find anything in the git
archive manpage to suggest this, nor any details about what
`pristine-tar` actually *does* to reason about this. A quick try shows
that gbp does indeed manage to reproduce the same tarball twice, but
that gives no guarantees about the same result with different git
versions and different machines, of course.

I was further confused by the gbp buildpackage manpage. It says:

> When gbp buildpackage doesn't find a suitable upstream tarball it will
> create one either using pristine-tar™ or git archive. These options
> determine how the tarball is created:

This is a bit confusing, I had expected that `git archive` would create a
tarball and then pristine-tar would be used to make it pristine. I later
read the `pristine-tar` manpage and noticed that pristine-tar actually
creates the files from git directly (combining upstream commit and the
pristine-tar commit), so no git archive involved.

Also, it says:

> --git-pristine-tar
>  Use pristine-tar when generating the upstream tarball if it doesn't
>  exist. If this mode is enabled the --git-upstream-tag,
>  --git-upstream-tree options have no effect.

More confusion: If gbp-buildpackage is supposed to generate a
pristine-tar delta when the tarball is first created from git, I would
think I needed to pass `--git-pristine-tar`, but *also*
`--git-upstream-tag` (or `--git-upstream-tree`) to allow gbp to locate
the right upstream commit to base the tarball on?

Later, I learned from the `pristine-tar` manpage that the pristine-tar
commit actually contains the git tree id of the commit used to create
it, so when recreating the tarball, pristine-tar can find the right
commit on its own.

But when creating the tarball the first time, `gbp buildpackage` *does*
require the upstream commit, right?

Or is the workflow to, on the first build for a new upstream version,
run buildpackage without `--pristine-tar` to create the tarball and then
run `pristine-tar` manually? I don't think so, since there is also `gbp
--git-pristine-tar-commit`:

> --git-pristine-tar-commit
>  Commit the pristine-tar delta to the pristine-tar branch if a new
>  tarball was generated and the pristine-tar data isn't already there.

Or does this option maybe disable `--git-pristine-tar` when there is no
pristine-tar data yet, and thus "reactivate" the `--git-upstream-*`
options?

If so (and if pristine-tar is still needed without upstream tarballs),
the workflow could be to always run:

gbp --git-pristine-tar --git-pristine-tar-commit 
--git-upstream-tag=v%(version)s

And that would then *create* a tarball and pristine-tar data when no
pristine-tar data is present yet, or *use* it (and ignore upstream-tag)
when pristine-tar data *is* present? A quick shows that this might
indeed work like this.

I would be grateful if you could clarify some of this for me. Updating
the docs would be even better, but maybe I can find some time to have a
look at that if you can clarify things here first :-)

Somewhat related: Is git archive even used at all? I ran 0.9.15 with
`--git-verbose`, which seems to print all individual git commands ran,
but I did not see git archive in there? Or is it just not printed?

Gr.

Matthijs


Bug#960485: lintian: false-positive no-dh-sequencer due to comments

2020-05-13 Thread Matthijs Kooijman
Package: lintian
Version: 2.72.0
Severity: minor

Hi,

lintian is incorrectly triggering no-dh-sequencer on my package (nml
0.5.1-2). The debian/rules file contains:

  # Use debhelper default for all targets (but some are overridden below).
  %:
  # Force the pybuild buildsystem, since there is also a
  # Makefile (which is used only for testing by this rules file).
  dh $@ --with python3 --buildsystem pybuild

Looking at the source for this check, I suspect that the comment before the dh
line prevents the dh sequence from being detected.

Gr.

Matthijs



Bug#905866: uscan: prefers watch files from a random dir over debian/watch

2020-05-12 Thread Matthijs Kooijman
Hey folks,

> (...)
> and then goes on to detail how this directory name checking works
> exactly. AFAICS, this directory name checking should protect against
> these stray watch files in most of the cases, but apparently it is not
> working.
> (...)
> AFAIU, "subdir" should have matched /^test(-+.)?/, which does not seem
> to be the case, so this check seems broken in uscan?

Turns out there was a bug with the directory checking. I submitted a fix
here:

https://salsa.debian.org/debian/devscripts/-/merge_requests/193

That MR also has a small change to the manpage that makes it a bit more
explicit that uscan works recursively by default.

>  - There currently seems to be no way to disable this behaviour at all,
>if it turns out to be problematic?

I've found that the `--watchfile` option disables recursive processing
and just processes the given file instead. So to just process the
package in the current directory, you can run `uscan --watchfile
debian/watch`.

>  - The most common usecase seems to be scanning for new versions of a
>single package, where this recursive scanning is not needed at all.
>Would it not make sense to just scan the current directory by
>default, and add an option to enable recursive scanning for usecases
>that need it?
I still think that the current default might not be ideal. However, I do
see the usecase of running uscan over a collection of debian package at
the same time.

Maybe the default could be changed to only scan the current directory
*if* it is a debian source tree, and default to recursive scanning if
not? That would support both the "Run on a single package" and "Run on a
collection of packages" usecases neatly?

More specifically, I would suggest:
 - Adding a `--no-recursive` option, which will always check only the
   current dir (and probably produce an error if no valid package and
   watchfile can be found). This might be implemented as an alias for
   `--watchfile debian/watch` maybe.
 - Adding a `--recursive` option, which ensures that recursive operation
   happens, even when the current directory is a valid source tree. This
   is what is the current default operation.
 - Specifying more than one of `--recursive`, `--no-recursive` and
   `--watchfile` is an error.
 - When none of `--recursive`, `--no-recursive` and `--watchfile` is
   specified, the default is to use `--no-recursive` when the current
   directory is a source tree, and `--recursive` otherwise.

One open question is what constitutes a "source tree" exactly for the
purpose of the default operation. The simplest (and most conservative)
is "Whenever a debian/ subdirectory exists", the most extensive is
probably "When a debian/changelog exists and can be parsed and
debian/watch exists".

I would tend to simplest rather than complex, since that is easier to
check and harder to break (e.g. a typo in the changelog causing uscan to
behave differently).

I *can* imagine that someone would have a debian directory in their
package collection they want to run uscan on (e.g. a debian and ubuntu
directory for splitting packages between those), so maybe "When
debian/changelog exists" is a good compromise here.

How does that sound?

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#960430: uscan: "Use only newest of same package"-check not working?

2020-05-12 Thread Matthijs Kooijman
Package: devscripts
Version: 2.20.3
Severity: minor

Hi,

while working on another problem, I noticed something weird in the code.

See 
https://salsa.debian.org/debian/devscripts/-/blob/master/lib/Devscripts/Uscan/FindFiles.pm#L167-193

There is a comment there, that says:

# Now process the watch files in order.  If a directory d has
# subdirectories d/sd1/debian and d/sd2/debian, which each contain watch
# files corresponding to the same package, then we only process the watch
# file in the package with the latest version number.

This seems to be implemented by sorting the watch files on version, and then
once you see a package that was previously already processed, skip it. However,
that skipping is implemented like this:


if (exists $donepkgs{$parentdir}{$package}) {
uscan_warn "Skipping $dir/debian/watch\n   as this package has already 
been found";
next;
}

IOW, it skips if a package was already processed that has both the same
$parentdir (e.g. the `foo-1.2.3` directory that contains the `debian` dir),
*and* the same $package (i.e. the package name parsed from `debian/changelog`).

This seems weird to me, I would think that just the same package name
should be enough. However, I could not find this behaviour mentioned in
the manpage at all, so I'm not exactly sure what the intended behaviour
is exactly.

To show this happens:

$ mkdir -p uscantest/test-1.2.3/debian
$ mkdir -p uscantest/test-2.0.0/debian
$ cd uscantest/
uscantest$ (cd test-1.2.3; dch --create --package test --newversion 1.2.3-1)
uscantest$ (cd test-2.0.0; dch --create --package test --newversion 2.0.0-1)
uscantest$ touch test-1.2.3/debian/watch test-2.0.0/debian/watch
uscantest$ uscan --report-status
uscan info: uscan (version 2.20.4~1.gbp0dd0c3) See uscan(1) for help
uscan info: Scan watch files in .
uscan info: Check debian/watch and debian/changelog in ./test-1.2.3
uscan info: package="test" version="1.2.3-1" (as seen in debian/changelog)
uscan info: package="test" version="1.2.3" (no epoch/revision)
uscan info: Check debian/watch and debian/changelog in ./test-2.0.0
uscan info: package="test" version="2.0.0-1" (as seen in debian/changelog)
uscan info: package="test" version="2.0.0" (no epoch/revision)
uscan info: ./test-2.0.0/debian/changelog sets package="test" version="2.0.0"
uscan info: ./test-1.2.3/debian/changelog sets package="test" version="1.2.3"
uscan info: Process watch file at: debian/watch
package = test
version = 2.0.0
pkg_dir = ./test-2.0.0
uscan info: Process watch file at: debian/watch
package = test
version = 1.2.3
pkg_dir = ./test-1.2.3
uscan info: Scan finished

It indeed processes the packages in descending version order, but does not skip
the lower version of the same package as the comment implies should happen.

I've tested this with current git master, so I used the most recently released
version number in the bug metadata.

Gr.

Matthijs



Bug#960419: dh_link: Add option to check for identical file contents before replacing

2020-05-12 Thread Matthijs Kooijman
Package: debhelper
Severity: wishlist

Hi,

in the openttd-opengfx package, I'm using dh_link to replace some
upstream-installed files with symlinks. To prevent accidentally linking
to the wrong file, I now explicitly diff the files before creating the
link. It would be nice if dh_link could handle that.

I can imagine an option that checks whether the to-be-created link and
the target already exist and if so, whether they are equal. I can
imagine four modes:

 1. Always create the link. This is the current behaviour.
 2. When both files exist and are not equal, do nothing. In all other
cases, create the link.
 3. When both files exist and are not equal, abort with an error. In all
other cases, create the link.
 4. When both files exist and are not equal, or either file does not
exist, abort with an error. Only if both files exist and are equal,
create the link.

I suspect that in most cases, the link will not overwrite any existing
files, so 2. is useful as a safeguard against accidentally overwriting
a non-identical file.

In my case, I'm letting the upstream Makefile first install files, and
then replace them with a symlink. In that case, I want to *only* replace
files and not create new links, so I would need 3.


One complication in implementing this would be cross-package links. For
implementing any of 2., 3. or 4. above, you need to be able to resolve
the target of the link. If that lives in another package, this is
tricky. I can imagine two flavours of this:
 - Links to other binary packages resulting from the currently built
   source package. Maybe dh can resolve those by looking in all
   available install dirs for the file. I guess this *might* result in
   multiple versions of the same file, which should then *all* be
   identical.
 - Links to unrelated binary packages (e.g. links to
   /usr/share/common-licenses). These could maybe be resolved against
   the build system (this is what I do for the license right now), but I
   don't think this can be made reliable, so this is probably better to
   not support at all.

Gr.

Matthijs



Bug#959665: git-buildpackage: please put a full stop after "New upstrem version X."

2020-05-11 Thread Matthijs Kooijman
Package: git-buildpackage
Followup-For: Bug #959665

Hi,

It does seem that having a full stop at the end of debian/changelogs is
conventional. However, this is already easy to configure locally. I had
this in my debian/gbp.conf for a long time:

  [import-orig]
  import-msg = New upstream release %(version)s.

I've since removed the dot again, since for *git* commit messages, it is
conventional to *not* have a full stop at the end of the first line and
I kept forgetting to add it for my Debian packages, so now my changelogs
also omit the full stop.

Also, it seems that Debian policy does not mention the full stop at all:
https://www.debian.org/doc/debian-policy/ch-source.html#debian-changelog-debian-changelog

Anyway, this is already easy to configure, so I wonder if this would be
worth changing?

Gr.

Matthijs



Bug#919259: systemd: (Security?) update breaks systemd (inside unprivileged container?)

2020-04-11 Thread Matthijs Kooijman
Hey folks,

I've upgraded this system to buster and it seems that either the new
kernel (4.19.0-8-amd64) or new lxc version (1:3.1.0+really3.0.3-8) has
fixed this problem: I can now again re-exec systemd in containers even
with lxc.cap.drop = sys_admin enabled.

I guess this issue could be closed? Feel free to do so if you think it
is appropriate.


Anyway, below is some more info I collected a long time ago but never
gotten around to cleaning up and sending. I'm including it here, in case
it is useful for anyone else running into the same.

Gr.

Matthijs

== Old debugging info below ==

When running systemd with debug loglevel (in /etc/systemd/system.conf),
I see the following on boot (from the console logfile, since journald
isn't running at that point yet):

Using cgroup controller name=systemd. File system hierarchy is at 
/sys/fs/cgroup/systemd.
Release agent already installed.

When reexecuting systemd, I get the following (from journalctl):

Using cgroup controller name=systemd. File system hierarchy is at 
/sys/fs/cgroup/systemd/../...
Release agent already installed.
Failed to create /../../init.scope control group: Operation not 
permitted
Failed to allocate manager object: Operation not permitted


The ../../init.scope is, I think, based on this file:

$ cat /proc/1/cgroup
10:freezer:/
9:pids:/../../init.scope
8:net_cls,net_prio:/
7:devices:/../../init.scope
6:blkio:/../../init.scope
5:memory:/../../init.scope
4:perf_event:/
3:cpu,cpuacct:/../../init.scope
2:cpuset:/
1:name=systemd:/../../init.scope

This is how it looks before and after the re-exec.

I'm not sure what this file looks like when systemd first starts in the
container, but I suspect the ../../ is not there yet, given the "File
system hierarchy is at /sys/fs/cgroup/systemd" log message, or maybe
systemd does not read it on initial startup?

On the host, the file looks like this:

$ cat /proc/1/cgroup
10:freezer:/
9:pids:/init.scope
8:net_cls,net_prio:/
7:devices:/init.scope
6:blkio:/init.scope
5:memory:/init.scope
4:perf_event:/
3:cpu,cpuacct:/init.scope
2:cpuset:/
1:name=systemd:/init.scope

When I look up the container's pid 1 on the host, it looks like this:

matthijs@tika:/etc/lxc$ cat   /proc/1755/cgroup
10:freezer:/lxc/template
9:pids:/init.scope
8:net_cls,net_prio:/lxc/template
7:devices:/init.scope
6:blkio:/init.scope
5:memory:/init.scope
4:perf_event:/lxc/template
3:cpu,cpuacct:/init.scope
2:cpuset:/lxc/template
1:name=systemd:/init.scope


When I start the container *with* CAP_SYS_ADMIN, the file inside the
container looks different:

matthijs@template:~$ cat /proc/1/cgroup | grep systemd
1:name=systemd:/init.scope

When I look up the container's pid 1 on the host, it looks like this:

matthijs@tika:/etc/lxc$ sudo cat /proc/507/cgroup | grep systemd
1:name=systemd:/lxc/template/init.scope

== New debug info ==

After the upgrade to buster, it seems that the scopes are now correct.
Inside the container *without* CAP_SYS_ADMIN, I now get:

$ cat /proc/1/cgroup |grep systemd
1:name=systemd:/init.scope


signature.asc
Description: PGP signature


Bug#880170: aptitude: Read error (-5: DATA_ERROR_MAGIC)

2020-04-07 Thread Matthijs Kooijman
Package: apt
Followup-For: Bug #880170

Hi,

I just also ran into this problem. It's on a stretch system, so with an
older apt, but maybe my observations help regardless.

I saw this error running `aptitude update`. It happened with the stretch
security updates translation file for contrib and non-free.

I read that emptying /var/lib/apt/lists might help, so I tried a light
version: I deleted only
security.debian.org_dists_stretch_updates_non-free_i18n_Translation-en
and security.debian.org_dists_stretch_updates_contrib_i18n_Translation-en
from that directory. I had expected these files to be somehow
problematic, so I saved them to restore them later to reproduce the
problem again.

Removing both files indeed made the `aptitude update` complete
succesfully. To my surprise these two files had been recreated, with
exactly the same contents as before. Another `aptitude update` succeeds,
so apparently something else changed (but I did not make a full backup
of the lists directory, unfortunately). So I could not reproduce the
problem by restoring these files manually, as I had planned.

Looking on the server, I see that a
http://security.debian.org/dists/stretch/updates/contrib/i18n/Translation-en.bz2
does exist, so the 404-theory stated previously might not be
accurate.

I also looked for the failing file in /var/lib/apt/lists/partial, but
there were no files in there when I looked.

Sorry I don't have anything more specific, but maybe some of this
helps...

Gr.

Matthijs

-- Package-specific info:

-- apt-config dump --

APT "";
APT::Architecture "amd64";
APT::Build-Essential "";
APT::Build-Essential:: "build-essential";
APT::Install-Recommends "1";
APT::Install-Suggests "0";
APT::Sandbox "";
APT::Sandbox::User "_apt";
APT::Authentication "";
APT::Authentication::TrustCDROM "true";
APT::NeverAutoRemove "";
APT::NeverAutoRemove:: "^firmware-linux.*";
APT::NeverAutoRemove:: "^linux-firmware$";
APT::NeverAutoRemove:: "^linux-image-4\.9\.0-6-amd64$";
APT::NeverAutoRemove:: "^linux-image-4\.9\.0-8-amd64$";
APT::NeverAutoRemove:: "^linux-headers-4\.9\.0-6-amd64$";
APT::NeverAutoRemove:: "^linux-headers-4\.9\.0-8-amd64$";
APT::NeverAutoRemove:: "^linux-image-extra-4\.9\.0-6-amd64$";
APT::NeverAutoRemove:: "^linux-image-extra-4\.9\.0-8-amd64$";
APT::NeverAutoRemove:: "^linux-signed-image-4\.9\.0-6-amd64$";
APT::NeverAutoRemove:: "^linux-signed-image-4\.9\.0-8-amd64$";
APT::NeverAutoRemove:: "^kfreebsd-image-4\.9\.0-6-amd64$";
APT::NeverAutoRemove:: "^kfreebsd-image-4\.9\.0-8-amd64$";
APT::NeverAutoRemove:: "^kfreebsd-headers-4\.9\.0-6-amd64$";
APT::NeverAutoRemove:: "^kfreebsd-headers-4\.9\.0-8-amd64$";
APT::NeverAutoRemove:: "^gnumach-image-4\.9\.0-6-amd64$";
APT::NeverAutoRemove:: "^gnumach-image-4\.9\.0-8-amd64$";
APT::NeverAutoRemove:: "^.*-modules-4\.9\.0-6-amd64$";
APT::NeverAutoRemove:: "^.*-modules-4\.9\.0-8-amd64$";
APT::NeverAutoRemove:: "^.*-kernel-4\.9\.0-6-amd64$";
APT::NeverAutoRemove:: "^.*-kernel-4\.9\.0-8-amd64$";
APT::NeverAutoRemove:: "^linux-backports-modules-.*-4\.9\.0-6-amd64$";
APT::NeverAutoRemove:: "^linux-backports-modules-.*-4\.9\.0-8-amd64$";
APT::NeverAutoRemove:: "^linux-tools-4\.9\.0-6-amd64$";
APT::NeverAutoRemove:: "^linux-tools-4\.9\.0-8-amd64$";
APT::VersionedKernelPackages "";
APT::VersionedKernelPackages:: "linux-image";
APT::VersionedKernelPackages:: "linux-headers";
APT::VersionedKernelPackages:: "linux-image-extra";
APT::VersionedKernelPackages:: "linux-signed-image";
APT::VersionedKernelPackages:: "kfreebsd-image";
APT::VersionedKernelPackages:: "kfreebsd-headers";
APT::VersionedKernelPackages:: "gnumach-image";
APT::VersionedKernelPackages:: ".*-modules";
APT::VersionedKernelPackages:: ".*-kernel";
APT::VersionedKernelPackages:: "linux-backports-modules-.*";
APT::VersionedKernelPackages:: "linux-tools";
APT::Never-MarkAuto-Sections "";
APT::Never-MarkAuto-Sections:: "metapackages";
APT::Never-MarkAuto-Sections:: "contrib/metapackages";
APT::Never-MarkAuto-Sections:: "non-free/metapackages";
APT::Never-MarkAuto-Sections:: "restricted/metapackages";
APT::Never-MarkAuto-Sections:: "universe/metapackages";
APT::Never-MarkAuto-Sections:: "multiverse/metapackages";
APT::Move-Autobit-Sections "";
APT::Move-Autobit-Sections:: "oldlibs";
APT::Move-Autobit-Sections:: "contrib/oldlibs";
APT::Move-Autobit-Sections:: "non-free/oldlibs";
APT::Move-Autobit-Sections:: "restricted/oldlibs";
APT::Move-Autobit-Sections:: "universe/oldlibs";
APT::Move-Autobit-Sections:: "multiverse/oldlibs";
APT::Periodic "";
APT::Periodic::Update-Package-Lists "1";
APT::Periodic::Unattended-Upgrade "1";
APT::Architectures "";
APT::Architectures:: "amd64";
APT::Compressor "";
APT::Compressor::. "";
APT::Compressor::.::Name ".";
APT::Compressor::.::Extension "";
APT::Compressor::.::Binary "";
APT::Compressor::.::Cost "0";
APT::Compressor::lz4 "";
APT::Compressor::lz4::Name "lz4";
APT::Compressor::lz4::Extension ".lz4";
APT::Compressor::lz4::Binary "false";
APT::Compressor::lz4::Cost "50";
APT::Compressor::gzip 

Bug#954672: openttd-opengfx: FTBFS: Error: (AttributeError) "module 'time' has no attribute 'clock'".

2020-03-23 Thread Matthijs Kooijman
reassign 954672 nml 0.4.5-2
thanks

Hi Lucas,

> > Error:(AttributeError) "module 'time' has no attribute 'clock'".
> > Command:  ['/usr/bin/nmlc', '-c', '-p', 'DOS', '-M', 
> > '--MF=ogfx1_base.grf.dep', '--grf', 'ogfx1_base.grf', 'ogfx1_base.nml']
> > Location: File "/usr/lib/python3/dist-packages/nml/generic.py", line 332, 
> > in print_progress

thanks for reporting. Seems this is really a bug in the nml compiler,
which uses time.clock, which was deprecated since Python 3.3 and removed
in 3.8. I'm also the maintainer of that package, so I'll make sure to
get this fixed.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#945945: dh-python: Should support DEB_BUILD_OPTIONS=terse

2019-12-01 Thread Matthijs Kooijman
Package: dh-python
Version: 4.20191017
Severity: wishlist

Hi,

it would be nice if dh-python could automatically handle
DEB_BUILD_OPTIONS=terse.

Debhelper does already have some machinery for this:

  /usr/share/perl5/Debian/Debhelper$ grep -r QUIET
  Dh_Lib.pm:  # make sure verbose is on. Otherwise, check DH_QUIET.
  Dh_Lib.pm:  } elsif (defined $ENV{DH_QUIET} && $ENV{DH_QUIET} ne "" || 
get_buildoption("terse")) {
  Dh_Lib.pm:  $dh{QUIET}=1;
  Dh_Lib.pm:  if (!$dh{QUIET}) {
  Buildsystem/ninja.pm:   if (!$dh{QUIET}) {
  Buildsystem/cmake.pm:   unless ($dh{QUIET}) {
  Buildsystem/autoconf.pm:if ($dh{QUIET}) {

Iow, it sets `$dh{QUIET}` when the build should be quiet, and some build
systems already check this, so it would make sense for dh-python to do the same.

pybuild already has a working `--quiet` option, so I included a patch below
that passes `--quiet` to `pybuild` when appropriate (this checks for
`--verbose`, since from looking at the code, I expect mixing `--verbose` with
`--quiet` will give funny results). The patch is made against an installed
version 4.20191017, so I hope it still applies to a master version as-is.

Gr.

Matthijs

And here's the patch:

--- a/Debian/Debhelper/Buildsystem/pybuild.pm
+++ b/Debian/Debhelper/Buildsystem/pybuild.pm
@@ -10,7 +10,7 @@ package Debian::Debhelper::Buildsystem::
 use strict;
 use Dpkg::Control;
 use Dpkg::Changelog::Debian;
-use Debian::Debhelper::Dh_Lib qw(error doit);
+use Debian::Debhelper::Dh_Lib qw(%dh error doit);
 use base 'Debian::Debhelper::Buildsystem';
 
 sub DESCRIPTION {
@@ -105,6 +105,10 @@ sub pybuild_commands {
push @options, '--dir', $dir;
}
 
+   if (not grep {$_ eq '--verbose'} @options and $dh{QUIET}) {
+   push @options, '--quiet';
+   }
+
my @deps;
if ($ENV{'PYBUILD_INTERPRETERS'}) {
push @result, ['pybuild', "--$step", @options];



Bug#945941: Lacking HTML encoding of debcheck results

2019-12-01 Thread Matthijs Kooijman
Package: qa.debian.org
Severity: normal

Hi,

for the "nml" package, I'm seeing some warnings from debcheck at [1]:

  Package declares a build time dependency on 'python3-all-dev:any' which is 
broken Syntax.
  Package declares a build time dependency on 'python3-pil ' which is broken 
Syntax.
  Package declares a build time dependency on 'python3-ply ' which is broken 
Syntax.

[1]: https://qa.debian.org/debcheck.php?dist=unstable=nml

At first glance, especially the latter two seem perfectly fine, making the
error confusing. On closer inspection, the HTML source for these lines looks
like:

  Package declares a build time dependency on 'python3-all-dev:any' which is 
broken Syntax.
  Package declares a build time dependency on 'python3-pil ' which is 
broken Syntax.
  Package declares a build time dependency on 'python3-ply ' which is 
broken Syntax.

So it seems that qa.debian.org embeds the debcheck results into the HTML
without any encoding, making the brackets be interpreted as HTML and the
contents effectively hidden.

In theory, this could be a security problem (XSS), though exploiting that
probably requires uploading a package with an XSS attack embedded in the
dependency line (which probably needs to be accepted by other tooling in the
process as well, so might even be impossible). Maybe other errors are more
exploitable, but the lack of anonymity in the uploads probably means that this
is really a security problem in practice.

Note that lack of support for such a  clause is the subject of
#816448, but the encoding should be solved separately (even when that bug is
also solved).

Solving this would probably be a matter of adding a `htmlspecialchars()` around
the output lines.

Gr.

Matthijs



Bug#942716: add patch

2019-10-21 Thread Matthijs Kooijman
Hey Matthias,

> patch at
> http://launchpadlibrarian.net/447871899/nml_0.4.5-1build2_0.4.5-1ubuntu1.diff.gz
Thanks for the report and the patch, looks good. I'll prepare an upload
with it, probably somewhere next week.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#942716: add patch

2019-10-21 Thread Matthijs Kooijman
Hey Matthias,

> Thanks for the report and the patch, looks good. I'll prepare an upload
> with it, probably somewhere next week.
I had another look upstream, seems they fixed it by falling back to
PILLOW_VERSION unconditionally, but I also noticed PIL.__version__ is
actually the recommended replacement (available since pillow 3.4.0,
which is < oldstable, so can probably be used unconditionally as well).

See https://github.com/OpenTTD/nml/issues/39 for the upstream
discussion.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#942372: git-buildpackage: Allow configuring vendor for dch

2019-10-15 Thread Matthijs Kooijman
Hey folks,

> It is possible to override these defaults by passing `--vendor` to dch,
> so it would be good if a value could be configured in gbp that is passed
> to `dch --vendor`. Ideally, the value could be configured globally, or
> per-package, I think.

I found the `dch-opt` configuration as a way to do this with the current
git-buildpackage version. E.g. in `~/.gbp.conf`:

[dch]
dch-opt = --vendor=debian

Alternatively, you can set the DEB_VENDOR environment variable to
(globally) change the default vendor that dpkg-vendor returns (as
documented in dpkg-vendor(1)).

I guess that pretty solves this request, though a dedicated option (or a
mention in the manpage) might still make it easier for people to figure
out how to do this.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#942372: git-buildpackage: Allow configuring vendor for dch

2019-10-15 Thread Matthijs Kooijman
Package: git-buildpackage
Version: 0.9.14
Severity: normal

Hi,

I'm running gbp on an Ubuntu system to build packages for Debian. By
default, dch (and thus also gbp dch) uses distribution and version
number defaults for Ubuntu, rather than Debian (based on the results of
dpkg-vendor).

It is possible to override these defaults by passing `--vendor` to dch,
so it would be good if a value could be configured in gbp that is passed
to `dch --vendor`. Ideally, the value could be configured globally, or
per-package, I think.

There might be other tools that need to be told about the vendor, but I
have not found them so far.

Gr.

Matthijs



Bug#942370: devscripts: [debchange] --release has undocumented exception for Ubuntu distribution name

2019-10-15 Thread Matthijs Kooijman
Package: devscripts
Version: 2.19.6
Severity: normal

When running dhc on Ubuntu with --release, it uses the upcoming ubuntu
version (eoan currently), rather than unstable as the distribution name
to put into the changelog. It took me a while to figure out that dch was doing
this, since this exception is undocumented.

The code has:

if ($vendor eq 'Ubuntu') {
# In Ubuntu the development release regularly changes, don't just copy
# the previous name.
$DISTRIBUTION = get_ubuntu_devel_distro();
} else {
$DISTRIBUTION = $changelog->{Distribution};
}

(From 
https://salsa.debian.org/debian/devscripts/blob/master/scripts/debchange.pl#L700-706)

But the manpage says:

   --release, -r
  Finalize  the  changelog for a release.  Update the
  changelog timestamp. If the distribution is set to
  UNRELEASED, change it to the distribu‐ tion from the
  previous changelog entry (or another distribution as
  specified by --distribution).  If there are no previous
  changelog  entries and an explicit distribution has not
  been specified, unstable will be used.

Without mentioning this exception.

Gr.

Matthijs


Bug#905866: uscan: prefers watch files from a random dir over debian/watch

2019-10-01 Thread Matthijs Kooijman
Package: devscripts
Version: 2.19.4
Followup-For: Bug #905866

Hey folks,

I'm also running into problems with uscan processing debian/watch files
in subdirectories. Looking at the manpage, it *seems* to say it only
scans `debian/watch`, but at the end of the manpage, under ADVANCED
FEATURES, it says:

   uscan actually scans not just the current directory but all its
   subdirectories looking for debian/watch to process them all.  See
   "Directory name checking".

Looking at the section "Directory name checking", it says:

  Similarly to several other scripts in the devscripts package, uscan
  explores the requested directory trees looking for debian/changelog
  and debian/watch files. As a safeguard against stray files causing
  potential problems, and in order to promote efficiency, it will
  examine the name of the parent directory once it finds the
  debian/changelog file, and check that the directory name corresponds
  to the package name. It will only attempt to download newer versions
  of the package and then perform any requested action if the directory
  name matches the package name.

and then goes on to detail how this directory name checking works
exactly. AFAICS, this directory name checking should protect against
these stray watch files in most of the cases, but apparently it is not
working.

To reproduce:

$ mkdir -p subdir/debian
$ (cd subdir; dch --create --package test --newversion 1.0.0-1)
$ touch subdir/debian/watch
$ uscan --report
$ uscan --report-status
uscan info: uscan (version 2.19.4) See uscan(1) for help
uscan info: Scan watch files in .
uscan info: Check debian/watch and debian/changelog in ./subdir
uscan info: package="test" version="1.0.0-1" (as seen in
debian/changelog)
uscan info: package="test" version="1.0.0" (no epoch/revision)
uscan info: ./subdir/debian/changelog sets package="test"
version="1.0.0"
uscan info: Process watch file at: debian/watch
package = test
version = 1.0.0
pkg_dir = ./subdir
uscan info: Scan finished

AFAIU, "subdir" should have matched /^test(-+.)?/, which does not seem
to be the case, so this check seems broken in uscan?



However, I'm actually wondering if this recursive scanning by default is
a good feature at all. Two considerations:

 - There currently seems to be no way to disable this behaviour at all,
   if it turns out to be problematic?
 - The most common usecase seems to be scanning for new versions of a
   single package, where this recursive scanning is not needed at all.
   Would it not make sense to just scan the current directory by
   default, and add an option to enable recursive scanning for usecases
   that need it?

Gr.

Matthijs



Bug#895982: devscripts: [uscan] don't look in VCS directories for d/changelog or make it optional

2019-10-01 Thread Matthijs Kooijman
Package: devscripts
Followup-For: Bug #895982

Hey folks,

it seems this was partly fixed in 2.19.6 by specifically ignoring .git
directories:

  https://salsa.debian.org/debian/devscripts/merge_requests/132

There is also a similar bug report to this one that is not limited to
VCS directories, but discusess the scanning of subdirectories in
general:

  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=905866

Gr.

Matthijs



Bug#941446: openttd: Upstream version 1.9.3 available

2019-10-01 Thread Matthijs Kooijman
Hi Georg,

> new upstream version 1.9.3 is available.
Thanks for the headsup, seems I missed that. I'll try to get this
uploaded ASAP.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#927079: libpam-script: Filters environment variables

2019-08-29 Thread Matthijs Kooijman
Package: libpam-script
Followup-For: Bug #927079


Hi,

you mentioned that libpam-script filters environment variables, but I
think this is not actually the case. Looking at the code, it only seems
to *add* a number of variables, not remove any.

For example I added the following line to my /etc/pam.d/sudo (just
before the common-auth line):

  auth optional pam_script.so dir=/etc/pam.d/lock-scripts

And then created /etc/pam.d/lock-scripts/pam_script_auth:

  #!/bin/sh
  env > /tmp/env

After running sudo, I get my entire environment in /tmp/env.

I suspect there might be other pam modules that might be clearing the
env, or maybe the application that calls the pam module?

Gr.

Matthijs



Bug#936027: libpam-script: Using "sufficient" prevents running post-auth modules

2019-08-29 Thread Matthijs Kooijman
Package: libpam-script
Version: 1.1.9-4
Severity: normal

Hi,

I've just installed libpam-script, and noticed it uses "sufficient" in
its pam config lines. This results in e.g. the following common-auth on
my system:

  # here are the per-package modules (the "Primary" block)
  authsufficient  pam_script.so
  auth[success=1 default=ignore]  pam_unix.so nullok_secure 
try_first_pass
  # here's the fallback if no module succeeds
  authrequisite   pam_deny.so
  # prime the stack with a positive return value if there isn't one
  # already; this avoids us returning an error just because nothing sets
  # a success code since the modules above will each just jump around
  authrequiredpam_permit.so
  # and here are more per-package modules (the "Additional" block)
  authoptionalpam_fscrypt.so
  authoptionalpam_cap.so
  # end of pam-auth-update config

IIUC, sufficient means to stop executing other modules if the script
succeeds. This is fine wrt other modules that do additional "primary"
authentication checks (e.g. only one of them needs to succeed), but
AFAICS this also prevents running additional modules (that typically run
after the primary modules (such as the fscrpt or cap modules above).

As you can see, the unix module uses a jump to skip any other primary
modules, rather than sufficient to skip *all* other modules. This is
something that pam-auth-update can apparently automatically handle.
Here's how this looks in /usr/share/pam-configs/unix:

  Name: Unix authentication
  Default: yes
  Priority: 256
  Auth-Type: Primary
  Auth:
  [success=end default=ignore]pam_unix.so nullok_secure 
try_first_pass
  Auth-Initial:
  [success=end default=ignore]pam_unix.so nullok_secure
  Account-Type: Primary
  Account:
  [success=end new_authtok_reqd=done default=ignore]  pam_unix.so
  Account-Initial:
  [success=end new_authtok_reqd=done default=ignore]  pam_unix.so

Note the "success=end", which I assume to be autoreplaced with an appropriate 
value.

Gr.

Matthijs

-- System Information:
Debian Release: buster/sid
  APT prefers disco-updates
  APT policy: (990, 'disco-updates'), (990, 'disco-security'), (990, 
'disco-backports'), (990, 'disco'), (50, 'testing'), (50, 'stable')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 5.0.0-25-generic (SMP w/4 CPU cores)
Kernel taint flags: TAINT_WARN
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), 
LANGUAGE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages libpam-script depends on:
ii  libc6 2.29-0ubuntu2
ii  libpam0g  1.3.1-5ubuntu1

libpam-script recommends no packages.

libpam-script suggests no packages.

-- no debconf information



Bug#935996: libpam-fscrypt: Should provide a manpage

2019-08-28 Thread Matthijs Kooijman
Package: libpam-fscrypt
Version: 0.2.4-2
Severity: normal

Hi,

I'm trying to use libpam-fscrypt, but I'm missing a manpage that
describes its usage.

I've found /usr/share/doc/fscrypt/README.md.gz which has some info on
the module, but is mostly limited to examples (for example, it is not
clear what parts the "auth" and "session" invocation of the module play
in the unlocking of encrypted files.

Having a complete manpage would be helpful in this sense.

Gr.

Matthijs



Bug#930264: modemmanager: --filter-policy=strict no longer respects blacklist and probes Arduinos

2019-06-09 Thread Matthijs Kooijman
Package: modemmanager
Version: 1.10.0-1
Severity: normal

Hi,

since a while, ModemManager is using the new "strict" filter policy.
Rather than probing all serial ports that are not blacklisted (which
causes problems with serial devices that do not like to be probed), it
now uses more specific rules about what devices are likely to be modems
and does not probe any others.

However, this strict policy does *not* use the blacklist (configured
through udev rules), which I believe is problematic.

This problem particularly surfaces when using Arduino serial devices.
These development boards enumerate as TTY ACM USB devices, which cause
the kernel to automatically create serial ports for them. However, in
their USB descriptors, these devices advertise support for AT commands
(class=2/subclass=2/protocol=1), which triggers the MM
FILTER_RULE_TTY_ACM_INTERFACE and makes it probe these devices.

Of course, the actual problem in this case is that these devices
misadvertise themselves in their USB descriptor. I've raised this issue
in the Arduino community [1] and hopefully this will be fixed in the
future, but this will not help for existing devices (whose firmware is
not automatically or easily updated). This means that there are a lot of
Arduino devices out there which are broken by the switch to strict.

Previously, these devices were handled by the blacklist, but now they
are again probed when they should not. I suspect that there might be
other devices that have a similar fate. Also, users (such as myself)
might have collected some udev rules with blacklists over time, which
now unexpectedly stop working.

It seems that disabling the blacklist in strict mode is not an oversight
from upstream, since they also offer a "paranoid" mode which is equal to
"strict" mode but with the blacklist and greylist (manual scan only)
enabled.

A potential fix is to use the paranoid policy rather than the strict
policy, or to explicitly enable the blacklist and/or greylist on top of
the strict policy. I tried the latter, which indeed prevents MM from
probing my Arduinos. I did this:

  $ cat /etc/systemd/system/ModemManager.service.d/override.conf
  [Service]
  Environment="MM_FILTER_RULE_TTY_BLACKLIST=1"

Note that upstream discourages using the blacklists in their
documentation:

> It is not recommended to use this option in normal setups as the
> blacklists may be obsoleted in future ModemManager versions (in favor
> of using the Strict policy as default).

However, I've raised this same issue upstream [2], asking them to reconsider
this position.


Since this issue is a regression (Stretch still has a working blacklist,
but the Buster version uses the swtrict policy), it would make sense to
me to still make this change in Buster, if the freeze policy allows
this.

[1]: https://github.com/arduino/ArduinoCore-avr/pull/92
[2]: https://gitlab.freedesktop.org/mobile-broadband/ModemManager/issues/127

Gr.

Matthijs

(Note that I'm running Ubuntu on this machine, but reporting this to
Debian since that's where I believe this should be fixed)

-- System Information:
Debian Release: buster/sid
  APT prefers disco-updates
  APT policy: (990, 'disco-updates'), (990, 'disco-security'), (990, 
'disco-backports'), (990, 'disco'), (50, 'testing'), (50, 'stable')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 5.0.0-16-generic (SMP w/4 CPU cores)
Kernel taint flags: TAINT_WARN
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), 
LANGUAGE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages modemmanager depends on:
ii  libc6  2.29-0ubuntu2
ii  libglib2.0-0   2.60.0-1
ii  libgudev-1.0-0 1:232-2
ii  libmbim-glib4  1.18.0-1
ii  libmbim-proxy  1.18.0-1
ii  libmm-glib01.10.0-1
ii  libpolkit-gobject-1-0  0.105-25
ii  libqmi-glib5   1.22.0-1.2
ii  libqmi-proxy   1.22.0-1.2
ii  libsystemd0240-6ubuntu5

Versions of packages modemmanager recommends:
ii  usb-modeswitch  2.5.2+repack0-2ubuntu1

modemmanager suggests no packages.

-- no debconf information



Bug#922251: live-build: support syslinux-efi as (additional) bootloader

2019-05-30 Thread Matthijs Kooijman
Hi Adrian,

I just updated the MR with the changes I discussed in my previous mail
(and also listed below). My previous mail also contained some responses
and questions, if you have some time to respond to those that would be
great.

> > >>> 3) Remove ldlinux.c32 for extlinux and syslinux (6fc68c1d)
I moved this into its own MR:
https://salsa.debian.org/live-team/live-build/merge_requests/26

> > Well, I think you should use something else.
> > modules32 is 9 characters long (not 8.3 compatible). What about module32
> > and module64? So that we can reuse the code in the iso side when
> > isolinux is updated to support hybrid isos in efi mode ?
> Good point, hadn't considered 8.3 compatibility. Singular "module32"
> sounds a bit weird to me, but it is probably clearer than something like
> "mods32" or even just "c32" (the latter seems reasonable, except that
> the "c64" directory would then still contain ".c32" files since that's
> what 64-bit syslinux-efi also uses...).
I changed to module32 and module64 now (also using module32 for the bios
modules, for consistency).

> > 6.1)
> > PATH syslinux command is still being written in capital letters in
> > share/bootloaders/syslinux-efi/syslia32.cfg and
> > share/bootloaders/syslinux-efi/syslx64.cfg while it should be written in
> > non-capital letters.
> Good catch, will fix that.
Fixed.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#920765: modemmanager: please package version 1.10 for buster

2019-05-03 Thread Matthijs Kooijman
Package: modemmanager
Followup-For: Bug #920765

Hey folks,

AFAICS modemmanager 1.10 is available in sid and buster currently, so
this bug can be closed?

Gr.

Matthijs



Bug#922251: live-build: support syslinux-efi as (additional) bootloader

2019-05-02 Thread Matthijs Kooijman
Hi Adrian,

thanks again for your thorough review. I'm responding to both your
e-mails inline below.

> >>> 3) Remove ldlinux.c32 for extlinux and syslinux (6fc68c1d)
> >>
> >> 3.1) Nice. I didn't know about those new syslinux architecture dependent
> >> files. As per the wiki you link (
> >> https://wiki.syslinux.org/wiki/index.php?title=Library_modules#All_Syslinux_variants_need_an_additional_ldlinux_module
> >> ) in the commit description I guess that even ldlinux.c32 wouldn't be
> >> used but ldlinux.e64 instead.
> > Actually, ldlinux.e64 is only for EFI. This commit only touches
> > BIOS-related stuff. I'm not sure how "architecture dependent files" are
> > relevant here, since this commit just removes some files which are
> > really superfluous (since the 'syslinux' and 'extlinux' commands used to
> > install the bootloader in binary_hdd make sure to copy their own version
> > of these files into the image).
> I'm revisiting this commit and I'm not sure this is right thing to do.
> If your pull request only affects syslinux-efi why do you even care
> about 32bit code?
True, this was just a cleanup I came across. It is sort of related due
to the next commit that moves all c32 modules into a subdirectory. I
wondered whether to also move ldlinux.c32 and whether leaving it would
cause problems with the CWD change in the EFI boot. I then concluded the
file was not used at all and just sidestepped the issue by removing it
entirely.

I did one more test and noted that the existence of these files do not
cause problems for syslinux-efi. I'm not sure it it makes sense to keep
them or not, but dropping this commit from this MR for now seems fine
with me.

> > Also, EFI supports 32 and 64 bit (hence modules32 and modules64), but
> > BIOS is (by definition, I think) only 32-bit, so I just used "modules".
> 
> Well, I think you should use something else.
> modules32 is 9 characters long (not 8.3 compatible). What about module32
> and module64? So that we can reuse the code in the iso side when
> isolinux is updated to support hybrid isos in efi mode ?
Good point, hadn't considered 8.3 compatibility. Singular "module32"
sounds a bit weird to me, but it is probably clearer than something like
"mods32" or even just "c32" (the latter seems reasonable, except that
the "c64" directory would then still contain ".c32" files since that's
what 64-bit syslinux-efi also uses...).

> >> 5.2) Anyway I don't think I like this code at all. I mean... you are
> >> supposed to create a new file named:
> >> scripts/build/binary_syslinux-efi
> >> and not hack around the existant one: binary_syslinux
> > 
> > That would make sense if I wanted syslinux-efi to be really indepenent,
> > but as I noted above, I wanted to make them share a single configuration
> > (and also allow syslinux-efi to be installed by itself). This seemed
> > like best way to achieve that, though alternative suggestions are
> > welcome :-)
> 
> Well, you could have a binary_syslinux_cfg file similar to the
> binary_loopback_cfg one or maybe binary_shared_cfg.
That would indeed make some sense. I had not really considered this
before, but did now. The problem with this approach is that both
binary_syslinux_cfg, binary_syslinux and binary_syslinux-efi would need
to duplicate code. At least they all need Install_Bootloader_Files,
which could be slightly generalized and moved into functions.

There is also a bunch of code which post-processes .cfg and splash.svg
files. This would be mostly needed in binary_syslinux_cfg only (since
the syslinux/syslinux-efi only contains a minimal config file that
includes the shared config file and needs minimal post-processing).

However, if binary_syslinux_cfg would install syslinux-shared and then
do all the post-processing, followed by binary_syslinux and/or
binary_syslinux-efi without any post-processing, you would:
 - lose the ability to override some the shared config files with
   bootloader-specific (e.g. syslinux/isolinux/extlinux)
 - lose backward-compatibility with existing live-build configs that do
   not have a syslinux-shared config directory but have all their config
   in the bootloader-specific directory (which is installed *after*
   configs are post-processed).

This can probably be fixed by further splitting the cfg step into an
install and post-processing step. You would then get:
 - binary_syslinux_cfg that installs config files verbatim (runs if any
   of syslinux/extlinux/isolinux/syslinux-efi is selected).
 - binary_syslinux that installs the modules and main config file for
   syslinux/isolinux/extlinux (runs if any of syslinux/extlinux/isolinux
   is selected, but not if only syslinux-efi is selected).
 - binary_syslinux-efi that installs the modules and main config file for
   syslinux-efi (runs if syslinux-efi is selected).
 - binary_syslinux_proces_cfg that post-processes all config files
   installed by previous steps (runs if any of
   syslinux/extlinux/isolinux/syslinux-efi is selected).

But I'm 

Bug#922251: live-build: support syslinux-efi as (additional) bootloader

2019-04-16 Thread Matthijs Kooijman
Hi Adrian,

thanks for your extensive review. I'll respond inline.

> I might take a look into your notes to implement grub-efi + secure boot
> in hdd img but... this might be in 2021 XD . Too busy at the moment.
Familiar sentiment. Would be nice to have it, though.

> What's your use case? What do you need to build live-build based hdd
> imgs? I'm curious.
I'm using live-build to build Webconverger (a kiosk OS booting into a
full-screen browser) and an image derived for that for Bizplay (a
digital signage system).

We need hdd images rather than isohybrid images, so the resulting
USB-disk can be written to. This is needed firstly to update the
bootloader configuration to change the kernel commandline (we use this
to customize the image, e.g. change the default homepage shown).
Secondly, the image uses git to manage the root filesystem and a running
image can update itself, but that again needs a writable filesystem.


> > As predicted by others in this bug report, my work does not enable
> > secure boot (which syslinux simply does not support), nor enable
> > syslinux-efi support in iso/isohybrid images (since syslinux-efi does
> > not support this, or at least it apparently does not work).
> 
> Nice. What you need to make sure is that you cannot choose syslinux-efi
> when trying to build iso/isohybrid images.
Yeah, that's checked by the "Improve bootloader configuration checks"
commit.

> > 0) backward incompatible change
> In the pull request there is a mention about a backward incompatible
> change. Can you please describe in what that change consists?
I've added one commit with a NEWS file that describes this issue in a
bit more detail. Does that clarify sufficiently?

Note that while writing the NEWS entry, it seemed more confusing to have
a bootloaders/syslinux-common directory as well as a syslinux-common
Debian package, so I ended up modifying my commits to use
"syslinux-shared" rather than "syslinux-common" (I'll also use the
former below).

> > 1) Refactor some bootloader scripts (969cdf4e)
> 
> 1.1) Nice idea the Get_Bootloader_Config_Dir function.
> Have you made sure that $BOOTLOADER variable is not used anywhere else
> in the code?
> Maybe you should make that variable local.
Good point. I guess I was following the style of other functions (that
dod not use local variables either), but that's no reason not to do it
here. I updated the PR with this change.

> 1.2) The function Install_Bootloader_Files does not seem to be right.
> If you are not going to reuse that function in more than one place (I'll
> check your other commits later) you might consider not having that
> function in the first place.
I am indeed planning to reuse the function. I've clarified this in the
commit message.


> Even if the Install_Bootloader_Files function was a good idea it's not
> well implemented commit-wise. You are performing two changes. One about
> moving the functionality to a function. Another one is chaning the
> comment about "two steps" to "three steps". It's hard to notice if you
> changed something other than that comment on that commit.
>
> I mean... this part should be splitted in two commits.
I'm usually a nitpick in this area, but here I decided for a bit more
simplicity. You're right, though. I ended up splitting this into three
commits, one for Get_Bootloader_Config_Dir, one for
Install_Bootloader_Files and one for the comment.

> > 2) Reduce config duplication in syslinux variants (bccd127b)
> 
> I know it's better the way you merge files there but I think the
> original idea was about having independent configuration files for each
> different media.
> 
> I mean there were not meant to be merged together in the first place.
The reason to do this, is that I want configuration files to be shared
between syslinux and syslinux-efi (which can co-exist in the same
image). If these would be duplicated everywhere, modifying the
bootloader config after an image is written to disk requires modifying
it in two places.

Note that syslinux-efi could still not have its own config files, but
refer to files contained in bootloaders/syslinux (without the need for
bootloaders/syslinux-shared), but then you can never have an image that
*only* contains syslinux-efi (which you can with the syslinux-shared
approach, since then you use syslinux-shared for the config and
syslinux-efi for the efi images).

> > 3) Remove ldlinux.c32 for extlinux and syslinux (6fc68c1d)
> 
> 3.1) Nice. I didn't know about those new syslinux architecture dependent
> files. As per the wiki you link (
> https://wiki.syslinux.org/wiki/index.php?title=Library_modules#All_Syslinux_variants_need_an_additional_ldlinux_module
> ) in the commit description I guess that even ldlinux.c32 wouldn't be
> used but ldlinux.e64 instead.
Actually, ldlinux.e64 is only for EFI. This commit only touches
BIOS-related stuff. I'm not sure how "architecture dependent files" are
relevant here, since this commit just removes some files which are
really 

Bug#922251: live-build: support syslinux-efi as (additional) bootloader

2019-04-16 Thread Matthijs Kooijman
Hey Thierry,

> Is there a chance that this work will be part of buster live-build
> package, or is it too late already ?
I'm not the maintainer of live-build, but given the freeze state that
buster is in, I highly doubt this will make it into buster.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#922251: live-build: support syslinux-efi as (additional) bootloader

2019-04-16 Thread Matthijs Kooijman
Hey Adrian,

[ About removing --templates from the manpage ]
> In that case IMO that commit should be in its own pull request and not
> the current one.
Done: https://salsa.debian.org/live-team/live-build/merge_requests/21

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#922251: live-build: support syslinux-efi as (additional) bootloader

2019-04-10 Thread Matthijs Kooijman
Hi Adrian,

> 1) What is the rationale behind removing the --templates option
> explanation on manpage?
I wondered about this option and looked around the source for it, but
could not find any implementation for it, so it seemed good to remove
the documentation for it.

Prompted by your question, I looked a bit further and found that it was
indeed removed:

https://salsa.debian.org/live-team/live-build/commit/7e633e77f24b6f5ab9a8b22d7d6cf6521454d638

> Note: I will make more comments about this bug later in the week.
Thanks!

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#922251: live-build: support syslinux-efi as (additional) bootloader

2019-04-08 Thread Matthijs Kooijman
Hi Luca & others,

I've been working on syslinux-efi support in the past weeks and just
submitted a MR with a working implementation, along with some small
bootloader-related cleanups and refactors:

https://salsa.debian.org/live-team/live-build/merge_requests/19

In the end, I opted to implement syslinux-efi rather than make grub-efi
work on hdd images, since that seemed easier and allows preserving the
existing bootloader config files in our project. Getting grub-efi to
work on hdd images might still be an interesting project that could be
implemented alongside syslinux-efi support, though I do not have any
specific purpose for it as of yet.

As predicted by others in this bug report, my work does not enable
secure boot (which syslinux simply does not support), nor enable
syslinux-efi support in iso/isohybrid images (since syslinux-efi does
not support this, or at least it apparently does not work).

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#778317: live-build: improve cache support for rebuilds with different arch

2019-03-22 Thread Matthijs Kooijman
Package: live-build
Version: 1:20190311
Followup-For: Bug #778317

Hi folks,

this bug still exists. The original report talks about erronously
reusing the same debootstrap cache when chaning architectures, but the
problem seems broader than that. E.g. when switching distributions,
you'll also get (more subtle) problems (in my case, parted from stretch
refusing to install to a chroot debootstrapped as buster or sid).

As stated before, (a solution to) this issue has two variants:

> 1) Users can easily switch architecture in their config and then
> unknowingly run into issues upon rebuilding.
> 2) For users building multiple images for different architectures, while
> you could just use a separate build directory for each, this would allow
> using the same build directory if desired, without having to throw away
> or rename the existing cached bootstrap, things would just work
> automatically.

Personally, I would think fixing 1) is important - preventing users from
running into issues when they change config but do not erase the cache. 2)
is probably more tricky to implement and seems less important.


As for implementation - I guess an obvious aproach would be to keep a file
inside the cache directory that indicates how this particular debootstrap cache
dir was created. This should include anything that can change the debootstrap
outcome, so an obvious way to do that would simply be to store the debootstrap
commandline used (and also check against it when restoring the cache). This
probably requires merging bootstrap_debootstrap and bootstrap_cache.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#924428: unblock: grfcodec/6.0.6-3

2019-03-12 Thread Matthijs Kooijman
Package: release.debian.org
Severity: normal
User: release.debian@packages.debian.org
Usertags: unblock

Please unblock package grfcodec

Version 6.0.6-3 adds a tiny patch to fix a serious bug (#922625).

unblock grfcodec/6.0.6-3

Thanks!

-- System Information:
Debian Release: 9.3
  APT prefers stable
  APT policy: (990, 'stable'), (500, 'unstable-debug'), (500, 'testing-debug'), 
(500, 'stable-debug'), (500, 'testing'), (500, 'oldstable'), (50, 'unstable'), 
(1, 'experimental-debug'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.11.0-1-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), 
LANGUAGE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
diff -Nru grfcodec-6.0.6/debian/changelog grfcodec-6.0.6/debian/changelog
--- grfcodec-6.0.6/debian/changelog 2018-05-10 21:42:34.0 +0200
+++ grfcodec-6.0.6/debian/changelog 2019-03-12 22:19:01.0 +0100
@@ -1,3 +1,11 @@
+grfcodec (6.0.6-3) unstable; urgency=medium
+
+  [ Jordi Mallach ]
+  * [e61a00b] Force build to abort upon endian_check failure. Thanks to
+Helmut Grohne for suggesting this fix (Closes: #922625)
+
+ -- Matthijs Kooijman   Tue, 12 Mar 2019 22:19:01 +0100
+
 grfcodec (6.0.6-2) unstable; urgency=medium
 
   * [4dce67c] Bump debhelper version to v11
diff -Nru grfcodec-6.0.6/debian/patches/endian_check_cpp_abort_on_ftbfs.patch 
grfcodec-6.0.6/debian/patches/endian_check_cpp_abort_on_ftbfs.patch
--- grfcodec-6.0.6/debian/patches/endian_check_cpp_abort_on_ftbfs.patch 
1970-01-01 01:00:00.0 +0100
+++ grfcodec-6.0.6/debian/patches/endian_check_cpp_abort_on_ftbfs.patch 
2019-03-12 22:19:01.0 +0100
@@ -0,0 +1,18 @@
+Description: Prevent infinite loop during build on endian_check failure
+Origin: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=922625
+Forwarded: https://github.com/OpenTTD/grfcodec/pull/1
+Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=922625
+
+Index: grfcodec/Makefile
+===
+--- grfcodec.orig/Makefile
 grfcodec/Makefile
+@@ -213,7 +213,7 @@ objs/$(ENDIAN_CHECK): src/endian_check.c
+ 
+ src/endian.h: objs/$(ENDIAN_CHECK)
+   $(_E) [ENDIAN] Determining endianness
+-  $(_C)objs/$(ENDIAN_CHECK) $(ENDIAN_PARAMS) > src/endian.h || rm 
src/endian.h
++  $(_C)objs/$(ENDIAN_CHECK) $(ENDIAN_PARAMS) > src/endian.h || { rm 
src/endian.h; exit 1; }
+ 
+ FORCE:
+ %_r: FORCE
diff -Nru grfcodec-6.0.6/debian/patches/series 
grfcodec-6.0.6/debian/patches/series
--- grfcodec-6.0.6/debian/patches/series2018-05-10 21:42:34.0 
+0200
+++ grfcodec-6.0.6/debian/patches/series2019-03-12 22:19:01.0 
+0100
@@ -1 +1,2 @@
 # Series of quilt patches
+endian_check_cpp_abort_on_ftbfs.patch


Bug#922251: live-build: support syslinux-efi as (additional) bootloader

2019-02-15 Thread Matthijs Kooijman
Hi Steve,

> In fact, one of the projects I've been trying to get to for a couple
> of years now is simplifying things by going the other way: using GRUB
> for everything and dropping syslinux on Debian media.

Hm, that's another interesting thought. I was aiming for syslinux, since
our current setup uses that (along with some custom configuration).
However, that seems to be impossible to work with secure boot (which
would be nice to have) and impossible to boot from optical media (which
for my personal case is not an issue).

I could imagine switching to grub completely (which means that this
issue changes from "add syslinux-efi" to "make grub-pc & grub-efi work
for hdd images"), though that's probably a bit more work for me and my
client.  I'll dig in a bit deeper to see how much work that would be.

Thanks for everyone's input so far!

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#922251: live-build: support syslinux-efi as (additional) bootloader

2019-02-15 Thread Matthijs Kooijman
Hey Luca,

> > So for the secure boot case in binary_grub-efi, what we do is that
> > if the signed monolithic EFI binaries are available we copy those
> > instead of building a new image.
> >
> > ...
> >
> > https://salsa.debian.org/live-team/live-build/blob/master/scripts/build/binary_grub-efi#L79
Aha! Turns out I was looking at an old version, I messed up my git
checkout apparently. That script indeed does what I would expect:
Install shim alongside grub and use signed grub to make shim load it.

> Ah silly me, I forgot something simple but quite fundamental: the point
> of syslinux is to avoid using grub entirely.

But indeed, I was aiming for syslinux, so none of this secure boot stuff
will actually work with syslinux.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#922251: live-build: support syslinux-efi as (additional) bootloader

2019-02-14 Thread Matthijs Kooijman
Hey Luca,

> At a quick glance it all sounds good to me, although I can't say I have
> a lot of experience with syslinux.
Ok.

> For feature parity, I'd encourage to look into supporting Secure Boot
> like the grub-efi implementation does, since we are preparing to ship
> that in Debian 10. It's not much extra work on top of adding the rest
> anyway.
Can you elaborate a bit on how grub-efi supports Secure Boot exactly? I
can't really find anything about this in the code?

Looking at build/scripts/binary_grub-efi and build/scripts/efi-image, I
see that a new efi firmware binary is built using grub-mkimage, so I
suppose that that image is not already signed, and there is nothing
suggesting that image is be signed at that time. Looking at binary_iso
there is also no reference to signing or secure boot.

AFAIU, to support secure boot, you need to sign the bootloader,
typically using a key from MS. I've read about the Shim bootloader,
which is signed and typically used to then load grub or other
bootloaders (signed by the Debian key or other keys included in Shim).
However, I can see no reference to shim either.

Looking at the grub package more closely, I *think* that it installs shim
alongside grub when using grub-install, but that is not used here?

Regardless, how would you suggest we "support Secure Boot" with
syslinux-efi exactly? AFAICT there is no syslinux-efi image available
signed with the MS key, and I suspect it is not signed with the Debian
key or any other key used by shim (also, since syslinux does not seem to
support key verification on kernels, I guess there is no secure way to
get syslinux booting under secure boot without compromising secure boot,
but I might be missing an important point about SB here...).

> > Since config sharing is easy and syslinux-efi is a matter of adding
> > some files to the existing image, it would make sense to add
> > syslinux-efi by default on normal syslinux hdd images (perhaps
> > adding a new option to disable this?).

I just noticed that lb config has a --bootloaders that supports
*multiple* bootloaders, so that would be perfect way to support this.
E.g. --bootloaders syslinux,syslinux-efi to have combined image (which
would also become the default for hdd images), or an explicit
--bootloaders syslinux or --bootloaders syslinux-efi to choose either
one individually.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#922251: live-build: support syslinux-efi as (additional) bootloader

2019-02-14 Thread Matthijs Kooijman
Hi Thomas,

> > it seems isohybrid can include a small FAT filesystem with the
> > bootloader files. [...]
> > https://www.syslinux.org/wiki/index.php?title=Isohybrid#UEFI
> 
> This describes the equipment of debian-live and debian-cd (DVD-*, BD-*,
> netinst) ISOs. See e.g. debian-live-9.5.0-i386-xfce.iso and its
> MBR partition table.
I'm a bit confused by your message. When you say "This", are you
referring to the syslinux isohybrid page?

> Debian and nearly all others use GRUB 2 for their EFI capable ISOs.
> See Knoppix 8.[12] for examples of SYSLINUX EFI in ISOs.
> 
> SYSLINUX EFI stuff is known not to boot from CD, DVD, or BD, but only from
> HDD, SSD, and alike.
Again, I'm confused. If syslinux-efi is known not to boot from
CD/DVD/BD, then why do they document how to include it into an isohybrid
image? Or does it then only work when booting the isohybrid image from a
HDD, rather than CD?

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#922251: live-build: support syslinux-efi as (additional) bootloader

2019-02-13 Thread Matthijs Kooijman
Package: live-build
Version: 1:20180925
Severity: wishlist

Hi folks,

currently, live-build seems to only support EFI systems using the
grub-efi bootloader, but not for netboot or hdd images (effectively only
for iso images, I believe).

Syslinux also has an EFI version, that can be installed through
the syslinux-efi package. It would be useful for live-build to support
this. I need this for a client, so I'm planning to implement support in
the coming weeks. This report serves to track progress and discuss
the implementation.

I've done some experiments by adding syslinux-efi to an existing image
manually (not with a full live-build image, but with my own hdd image
that at least uses live-build for building the image, so should be
representative in this area). This shows that adding syslinux-efi is
fairly easy. The existing FAT partition can function as an ESP (EFI
System Partition) as it is now.

Installing the bootloader is a matter of dumping some files in the
EFI/BOOT directory. This lets the bootloader be detected as a fallback
bootloader, which is AFAIU intended for removable media. Syslinux needs
some additional files (ldlinux, additional modules, configuration) which
can live in that same directory.

Both 32-bit and 64-bit EFI can be supported at the same time, by
installing both versions of syslinux-efi (named bootx64.efi and
bootia32.efi respectively). One caveat is that syslinux needs different
.c32 modules for both architectures (though they are both named .c32 and
are explicitly referenced in the config file). This means either
duplicating the bootloader configuration file for 32 and 64 bit (which
hinders modifications to the config), or putting the modules in
subdirectories and using the PATH config directive to point to either
directive before including the common configuration. I have not tried
this latter approach but it is described here (though currently
syslinux.org seems to be unavailable, I tried the Google cache):

https://www.syslinux.org/archives/2014-August/022545.html

(subject: syslinux efi configuration file name proposal)

I've also tried to let the syslinux-efi config file include the normal
syslinux configuration file (or at least the bulk that is in live.cfg),
which seems to work just fine.

Since config sharing is easy and syslinux-efi is a matter of adding some
files to the existing image, it would make sense to add syslinux-efi by
default on normal syslinux hdd images (perhaps adding a new option to
disable this?). This also seems to hold for ISO9660 images, where
it seems isohybrid can include a small FAT filesystem with the
bootloader files. This might need some additional work to generate the
filesystem image and/or pass options when generating the iso. See:

https://www.syslinux.org/wiki/index.php?title=Isohybrid#UEFI


Using syslinux-efi exclusively (e.g. passing it to --bootloader and not
installing normal syslinux) might also be an extra option. However, I'm
not much interested in this case, so I will likely not implement it
(I'll try not to make it too hard to add it later).


In terms of code, this is probably best implemented in the existing
binary_syslinux script. The bulk of what needs to happen is essentially
copying bootloader files from config/bootloaders into binary, taking
care to resolve symlinks. I'm planning to put the code that does that
into a shell function, so it can be called at the current place and then
a second time for syslinux-efi later.

I think it would be good to copy files *from*
config/bootloaders/syslinux-efi-addon or something similar, to leave
config/bootloaders/syslinux-efi available for an EFI-only image. These
two directories would be identical in terms of syslinux binary files,
but the configurations would differ (the addon would include the normal
boot/syslinux/syslinux.cfg, while the standalone version would contain
the complete config).

I haven't really looked at the iso version yet (which is also not my
primary interest, but I think it would be good to handle as well).

Happy to hear any thoughts :-)

Gr.

Matthijs



Bug#919259: systemd: (Security?) update breaks systemd (inside unprivileged container?)

2019-01-16 Thread Matthijs Kooijman
severity 919259 normal
found 919259 232-25
retitle 919259 Reexecuting fails in containers without CAP_SYS_ADMIN
thanks

Hey Michael,

> My suggestion would be to roll back to 232-25+deb9u1 and then gradually
> upgrading to deb9u2, deb9u2 ... deb9u3
Yeah, that was my intention. I discovered some interesting things.

 - On my host system, systemd is now also upgraded without problems.
 - Restarting a container works just fine, even with deb9u8. However,
   the problem occurs when re-execing (e.g. systemctl daemon-reexec).
 - Downgrading to deb9u1 and re-execing also breaks, so this is not
   broken by the upgrade that happened this week.
 - Looking in the logs, I found that the last time re-exec happened
   (succesfully) was on 2017-09-12. It seems that was from a manual
   upgrade, so I am not sure what version that was exactly. From old
   unattended upgrade e-mails, I *suspect* it was deb9u1.
 - Looking through my config git history, I did not find any seemingly
   relevant changes in the lxc config since 2017. However, I think I did
   upgrade from jessie to stretch since then (or maybe just before then,
   but I probably didn't restart the containers and/or system until
   later).
 - For good measure, I also tested the original 232-25 version, which
   also breaks.
 - When I remove `lxc.cap.drop = sys_admin` from the lxc config, reexec
   works fine again.

So it seems that *some* lxc upgrade since 2017 broke this. What is
broken is re-execing systemd inside a container running without
CAP_SYS_ADMIN.

I'm not sure if this means this bug should be against lxc instead, but
until we know more, I guess keeping it against systemd would be good.

Since booting still works (and this issue can be worked around be
rebooting the container), I'd say this issue can be downgraded
from important to normal. I've went ahead and did this, feel free to
revert if you feel otherwise.

Going forward, I guess it would be good to investigate why the re-exec
fails, based on the error messages shown:

  systemd[1]: Failed to create /../../init.scope control group: Operation not 
permitted
  systemd[1]: Failed to allocate manager object: Operation not permitted

One interesting question is also why the initial startup does *not* fail, but
the re-exec does.

I'm out of time again for now. I'll have a closer look at what this init.scope
control group is exactly and how systemd handles it on normal startup. Any
additional thoughts are welcome :-)

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#910990: openttd FTCBFS: uses the build architecture pkg-config

2018-11-19 Thread Matthijs Kooijman
Hi Helmut,

On Sun, Nov 18, 2018 at 04:27:29PM +0100, Helmut Grohne wrote:
> Hi Matthijs,
> 
> On Sat, Nov 17, 2018 at 09:28:54PM +0100, Matthijs Kooijman wrote:
> > Thanks for testing and provide a patch. I've included in the build, and
> > verified it works. I did run into a problem running on a system where
> > buildtools.mk did not exist, due to the % wildcard rule (see
> > https://lists.debian.org/debian-mentors/2011/10/msg00308.html and
> > https://salsa.debian.org/openttd-team/openttd/commit/2a60f8c976c12f3338655ba4c5130213987dde9a
> >  )
> 

> Dang. I didn't think of how make would handle absence.
Me neither, but at least the error message was Googlable :-)

> So buildtools.mk will always exist in buster and later releases. You
> only get problems when backporting to stretch or older (and we don't
> care about cross compilation there). The -include was meant to make it
> backport-friendly, but it ultimately failed doing exactly that.

I noticed it because git-buildpackage first cleans my git checkout to
generate a source package for pbuilder, which I do on a stable system.

> How about adding an empty rule for /usr/share/dpkg/buildtools.mk? Would
> that work for you?
That would work, but I opted for replacing the wildcard rule with an
explicit list. See the links I noted:

https://lists.debian.org/debian-mentors/2011/10/msg00308.html
https://salsa.debian.org/openttd-team/openttd/commit/2a60f8c976c12f3338655ba4c5130213987dde9a

So I think I'm good, I just wanted to give you a heads up for possible
future patches :-)

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#906906: icu FTCBFS: fails linking -licu-le-hb in the native pass

2018-11-17 Thread Matthijs Kooijman
Hey folks,

> Meanwhile, Matthijs can you tell us how the OpenTTD layout work goes?
> May you have any ETA from its upstream? It would be good to drop
> icu-le-hb very soon.
As already noted elsewhere, I just uploaded an OpenTTD version without
the iculx and icu-le-hb dependency. Not sure how that affects this bug
exactly, but I'm confident you guys will figure that out :-)

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#910990: openttd FTCBFS: uses the build architecture pkg-config

2018-11-17 Thread Matthijs Kooijman
Hi Helmut,

> openttd fails to cross build from source, because it uses the build
> architecture pkg-config. The build system expects the packaging to pass
> --pkg-config to use a triplet-prefixed pkg-config. Doing so is
> sufficient to make openttd cross buildable. Please consider applying the
> attached patch.

Thanks for testing and provide a patch. I've included in the build, and
verified it works. I did run into a problem running on a system where
buildtools.mk did not exist, due to the % wildcard rule (see
https://lists.debian.org/debian-mentors/2011/10/msg00308.html and
https://salsa.debian.org/openttd-team/openttd/commit/2a60f8c976c12f3338655ba4c5130213987dde9a
 )

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#898571: build dependency cycle between icu and icu-le-hb

2018-11-17 Thread Matthijs Kooijman
Hi folks,

I just uploaded openttd 1.8.0-2, which no longer has the ICU layout API
enabled. This should clear the way for dropping iculx and icu-le-hb.

This change makes at least Arabic and Hebrew support in OpenTTD pretty
much unusable. However, some additional investigation suggests that
Harfbuzz and ICU should provide enough building blocks to implement
something better again. See upstream github for discussion about that:

https://github.com/OpenTTD/OpenTTD/issues/6922

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#913509: openttd FTBFS with ICU 63.1

2018-11-17 Thread Matthijs Kooijman
Hi László,

thanks for your investigation and patch. I ended up backporting the
upstream patches for the issue, which turned out to be identical to your
patch :-)

I've just uploaded a version with this patch, as well as a bunch of
other pending changes.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#913863: libicu63: Versioned Breaks against openttd too strict

2018-11-17 Thread Matthijs Kooijman
Hi László & Ivo,

> >  That's too much time for blocking the ICU transition. I'll clear this
> > barrier of the openttd NMU in some days. :-/
Ah, I hadn't realized this was blocking a transition. Because of that, I
tried a bit harder to find time, and just uploaded a fixed version to
unstable.

> I added a removal hint for openttd, to allow icu to migrate to testing. As
> soon as a fixed version of openttd is ready, it can come back.
Is anything needed for making openttd come back to testing, or will an
upload that fixes the relevant bugs sufficient?

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#913863: libicu63: Versioned Breaks against openttd too strict

2018-11-16 Thread Matthijs Kooijman
Hi Lásló,


>  That's too much time for blocking the ICU transition. I'll clear this
> barrier of the openttd NMU in some days. :-/
Prompted by your other mail, I found some time in the airport today to
look at the ICU build issue in OpenTTD. I still have a few minor things
to include, but I should be able to upload a fixed version somewhere
this weekend.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#913863: libicu63: Versioned Breaks against openttd too strict

2018-11-16 Thread Matthijs Kooijman
Hey László,

> Matthijs working on a normal package upload? At least he answers my
> mails quick and I thought he can apply my patch and upload it as
> 1.8.0-2 package version.
Ack, it's on my list. I'm a bit busy these weeks, but I expect to find
some time in the next 2/3 weeks.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#898571: build dependency cycle between icu and icu-le-hb

2018-11-15 Thread Matthijs Kooijman
Hi Laszlo,

> I do Cc its maintainer Matthijs and if he acknowledges I will drop
> libiculx and icu-le-hb altogether.

Yeah, I think that dropping icu-le-hb is the best course forward. I want
to doublecheck that that does not have any unintended side effects. Ok
if I get back to you about this in 2/3 weeks? Or would you rather act on
this before then?

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#897233: openttd: Replace ICU ParagraphLayout usage

2018-10-04 Thread Matthijs Kooijman
Hey folks,

I've (finally) opened an upstream bug for this to track progress:
https://github.com/OpenTTD/OpenTTD/issues/6922

If this cannot be fixed upstream in a timely manner, one additional
option would be to simply not compile OpenTTD with ICU layout support.
There is already a simpler fallback layouter in place that is probably
satisfactory for western languages, but probably provides bad results
for RTL languages, or languages where word-breaking works differently.

I'm wouldn't be too happy with this, though I'm not sure how much users
would really be affected (From a quick glance, popcon does not collect
locales...).

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#906906: icu FTCBFS: fails linking -licu-le-hb in the native pass

2018-10-04 Thread Matthijs Kooijman
> Meanwhile, Matthijs can you tell us how the OpenTTD layout work goes?
> May you have any ETA from its upstream? It would be good to drop
> icu-le-hb very soon.
There's not much progress here. I haven't got time to really work on
this, and haven't gotten around to pushing at this since we first
spotted the issue a few months ago. Previously I've discussed this with
upstream through IRC, but this week I opened up an upstream bug report
for this, but not much is happening there yet:

https://github.com/OpenTTD/OpenTTD/issues/6922

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#846320: Duplicate consolekit assertion bugs

2018-09-25 Thread Matthijs Kooijman
package consolekit
merge  898411 846320
thanks

Hi,

I found these two bugs are actually about the same issue, so I went
ahead and merged them.

I'm also seeing this issue on 0.4.6-6. Looking around, I found an old
bug report at Fedora which suggests this might be caused by ConsoleKit
calling udev-acl wrongly: https://bugzilla.redhat.com/show_bug.cgi?id=611159

That report suggests that ConsoleKit calls commands in run-seat.d with a
plain action argument, while udev-acl expects an --action option.

This is confirmed by my log output:

Sep 24 06:00:08 tika udev-acl.ck[21215]: g_slice_set_config: assertion 
'sys_page_size == 0' failed
Sep 24 06:00:08 tika console-kit-daemon[6695]: missing action

And by running it manually:

$ ls -l /usr/lib/ConsoleKit/run-seat.d/udev-acl.ck
lrwxrwxrwx 1 root root 18 Jan 25  2017 
/usr/lib/ConsoleKit/run-seat.d/udev-acl.ck -> /lib/udev/udev-acl

$ /lib/udev/udev-acl foo
(process:12937): GLib-CRITICAL **: g_slice_set_config: assertion 
'sys_page_size == 0' failed
missing action

OTOH, it seems that the assertion might be unrelated to this, as it even occurs
with `--help`, so the assertion might be a bug in udev, while the "missing
action" error might be a bug in the consolekit package (since that suplies the
udev-acl.ck symlink).

$ /lib/udev/udev-acl --action=foo --user=123
(process:12947): GLib-CRITICAL **: g_slice_set_config: assertion 
'sys_page_size == 0' failed

$ /lib/udev/udev-acl --help
(process:12989): GLib-CRITICAL **: g_slice_set_config: assertion 
'sys_page_size == 0' failed
Usage: udev-acl --action=ACTION [--device=DEVICEFILE] [--user=UID]

However, after writing this I read that consolekit is no longer maintained and
mostly replaced by systemd-logind, so I ended up simply removing the consolekit
package which "fixes" this log message.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#898356: git-buildpackage: Multiple --git-pbuilder-options do not stack

2018-05-17 Thread Matthijs Kooijman
Hi Guido,

>   --git-pbuilder-options-append="option1" 
> --git-pbuilder-options-append="option2"
> 
> so you can decide on a per option basis if you want to append to the
> currently set value. Each "--git-pbuilder-options" would reset the whole
> thing and start over so:
> 
>   --git-pbuilder-options="foo" --git-pbuilder-options-append="option1" 
> --git-pbuilder-options-append="option2"
Ah, that is *exactly* what I meant. I just named the options differently
and apparently failed to express my intent. Thanks for providing some
clarifying examples :-)

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#898356: git-buildpackage: Multiple --git-pbuilder-options do not stack

2018-05-17 Thread Matthijs Kooijman
Hi Guido,

> > My suggestion of adding a --git-append-pbuilder-option could solve both
> > usecases:
> >  - you can use --git-pbuilder-options on the commandline to override all
> >previously set options, including in gbp.conf
> >  - you can use --git-append-pbuilder-option to extend any previously set
> >options.
> 
> You would also need to define how option stack over the various gbp.conf
> files. I don't think we want to go down that road.
Hm? But isn't there some order defined already? Currently each time the
option is given it overrides the previous value, so that is similarly
dependent on the parse order (perhaps even more, since when you only use
append, the order is irrelevant). Or am I misunderstanding what you mean
by "stack"?

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#898356: git-buildpackage: Multiple --git-pbuilder-options do not stack

2018-05-10 Thread Matthijs Kooijman
Hi Guido,

> > When passing multiple options on the commandline, this is (for me)
> > unexpected but easily fixed by passing all needed options into a single
> > --git-pbuilder-options, but when a gbp.conf file also uses
> > "pbuilder-options", these are also overriden by the commandline, which
> > is harder to fix (which needs copying the options from the config file
> > to the commandline).
>
> This in fact is intended since otherwise there'd be now way to override
> what's in gbp.conf (or rather in the several gbp.conf's parsed).
So you're suggesting to let multiple commandline options stack, but let
any commandline options override the config file option? That would seem
a bit confusing to me. Also, that would not allow using the commandline
to add additional options.

My suggestion of adding a --git-append-pbuilder-option could solve both
usecases:
 - you can use --git-pbuilder-options on the commandline to override all
   previously set options, including in gbp.conf
 - you can use --git-append-pbuilder-option to extend any previously set
   options.

At the same time, it would also keep backward compatibility as a bonus.

(I used a singular, perhaps --git-append-pbuilder-options makes more
sense, though I guess that depends on whether argument splitting is
applied or not)

> There are several levels of quiting going on, gbp itself does not do
> much but git-pbuilder and pbuilder do.
Good call. I did a bit of digging, turns out it is cowbuilder that
messes up, so this is entirely unrelated to this bug report. See
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=898366

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#898366: cowbuilder: Does not support multiple --extrapackages options

2018-05-10 Thread Matthijs Kooijman
Package: cowbuilder
Version: 0.87+b1
Severity: normal

Hi,

currently, cowbuilder seems to support only one occurance of the
--extrapackages option, where each subsequent occurance overrides the
previous one.

E.g.:

sudo -E cowbuilder --build --extrapackages=a.deb --extrapackages=b.deb some.dsc

results in:

I: forking: pbuilder build --buildplace
  /var/cache/pbuilder/build/cow.22054 --buildresult
  /var/cache/pbuilder/result/ --mirror http://ftp.nl.debian.org/debian
  --distribution sid --extrapackages b.deb --no-targz
  --internal-chrootexec 'chroot /var/cache/pbuilder/build/cow.22054
  cow-shell' some.dsc

Which only has the latter package.

The source code (0.87) seems to agree on this:

} else if (!strcmp(long_options[index_point].name,
 "extrapackages")) {
  /* this is for qemubuilder and cowbuilder (adds cowdancer) */
  pc.extrapackages = strdup(optarg);
}

However, pbuilder allows specifying this option multiple times:

--extrapackages [packages to add]
  Adds packages specified as an addition to the default, which is 
build-essential by default.  This is used in build and create (after success‐
  fully creating the initial chroot) and update.

  The packages should be specified as a space-delimited list, or by 
specifying --extrapackages multiple times.


It would make sense for cowbuilder to support this as well, or if not
at least raise an error when multiple --extra-packages options are
given.

Gr.

Matthijs


Bug#898356: git-buildpackage: Multiple --git-pbuilder-options do not stack

2018-05-10 Thread Matthijs Kooijman
Hi Guido,

I noticed one more related thing, also possibly a bug. It seems that the
arguments to --git-pbuilder-options are further processed. I tried
running this command:

gbp buildpackage --git-pbuilder-options=--extrapackage=/foo/a.deb\ 
--extrapackage=/foo/b.deb\ --bindmounts=/foo

Which resulted in this pbuilder command:

I: forking: pbuilder build --debbuildopts  --debbuildopts
  --bindmounts /foo --buildplace /var/cache/pbuilder/build/cow.31796 
--buildresult /tmp
  --extrapackages /foo/b.deb --no-targz --internal-chrootexec
  'chroot /var/cache/pbuilder/build/cow.31796 cow-shell'
  /tmp/openttd-opengfx_0.5.4-2.dsc


Interesting is that the --bindmounts and --extrapackages are no longer
adjacent, and that only the second --extrapackages is present. This
behaviour is even more surprising to me, and I'd think this would
certainly constitute a bug (at least the latter).

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#898356: git-buildpackage: Multiple --git-pbuilder-options do not stack

2018-05-10 Thread Matthijs Kooijman
Package: git-buildpackage
Version: 0.9.8
Severity: normal

Hi Guido,

I was using the --git-pbuilder-options option to gbp-buildpackage today,
and was surprised that multiple options do not stack. Instead, each
--git-pbuilder-options passed overrides the previous one, so only the
last one is effective.

When passing multiple options on the commandline, this is (for me)
unexpected but easily fixed by passing all needed options into a single
--git-pbuilder-options, but when a gbp.conf file also uses
"pbuilder-options", these are also overriden by the commandline, which
is harder to fix (which needs copying the options from the config file
to the commandline).

I'm not sure if this is really a bug, or more a feature request (since
it probably works as intended, except that that isn't quite ideal).

Ideally, multiple --git-pbuilder-options options would just be appended,
though I'm not sure if anyone would rely on the old behaviour. A
possible fix would be to add a --git-append-pbuilder-option flag, which
adds it argument to the value of --git-pbuilder-options.

Gr.

Matthijs



Bug#894159: OpenTTD, icu and ParagraphLayout

2018-05-01 Thread Matthijs Kooijman
Hi László,

> > So you're saying I could upload OpenTTD now, and it will be recompiled
> > with icu60 once that is uploaded to Sid? Or should I wait a few days?
>   Well, the transition update and _maybe_ the real start is hours soon. But
> as I see, OpenTTD might be uploaded and built, installed and tested until
> ICU 60.2 is allowed to be uploaded to Sid. There's a package which needs to
> be patched for that and its discussion may delay the actual transition.
Ok, so I went ahead and uploaded my OpenTTD package to unstable just
now, which should be able to compile against icu57 now and icu60 later.

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#894159: OpenTTD, icu and ParagraphLayout

2018-04-30 Thread Matthijs Kooijman
Hi László,

> > First off: I have a pending new upstream version of OpenTTD that I'd
> > like to upload, but I don't want to interfere with this transition.
> > Should I hold it off, or do we expect that resolving these issues will
> > take a while and should I just upload it now (and have it build against
> > the current icu version)?
>The transition will happen soon. I'm re-recompiled almost everything to
> really start it.

> I'm biased a bit. The current OpenTTD version in Sid compiles fine. If you
> can make the package available before upload or can check the compilation
> yourself with the experimental version of ICU then it would be good.
> I think there's still a day or two before the actual transition so I think
> you can upload OpenTTD after the mentioned test above.
I just compiled the new OpenTTD version against libicu60 from
experimental and that compiles fine. At first glance, internationalized
word-wrapping and right-to-left text looks fine (though I don't read any
such language, the results seem identical to what icu57 produced, modulo
some subtle space changes).

So you're saying I could upload OpenTTD now, and it will be recompiled
with icu60 once that is uploaded to Sid? Or should I wait a few days?

> > Neither me or upstream has much experience in this field, perhaps you
> > have a different suggestion for an alternative?
>   I don't know any other alternative. Only OpenTTD uses the Paragraph Layout
> API and it makes me wonder what other solution the other projects use? I
> may think other games like Lincity-NG[3] also need internationalized text
> placement and/or LibreOffice still need to handle this as well. Do these
> have an alternative solution?

Thanks, we'll have a closer look. I've created a new bug to further
track this, since it seems this will not block this particular
transition. Let's continue discussion here:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=897233

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#897233: openttd: Replace ICU ParagraphLayout usage

2018-04-30 Thread Matthijs Kooijman
Package: openttd
Version: 1.7.1-1
Severity: normal

Hi,

OpenTTD currently uses the ICU ParagraphLayout API for doing
internationalized wordwrapping. This usage is a problematic, since
ICU deprecated and removed their layout API for being buggy and
unmaintained. The ParagraphLayout API that OpenTTD uses is still present
in ICU, but it relies on the removed Layout API. This gap can be be
filled using the icu-le-hb library, which implements the layout API on
top of the Harfbuzz library.

However, it seems icu-le-hb is more of a proof of concept and not really
maintained either. Additionally, because ParagraphLayout is in ICU and
icu-le-hb depends on ICU, this gives an undesirable circular dependency
which complicates updating to new ICU versions.

See https://bugs.debian.org/894159 for more info on this, in particular
https://bugs.debian.org/894159#45 and onwards.


It seems it would be beneficial to move away from the ICU
ParagraphLayout API, so it can be removed (OpenTTD is the only user in
Debian), but also because it has indeed proven to be buggy in the past
(OpenTTD talks about ICU-related crashes, though this was before the
icu-le-hb implementation).

In the above linked bug, the following was written:

> > Another solution is of course to disable ParagraphLayout. László also
> > asked if OpenTTD, being the only user of this API, could migrate to
> > another solution. I've discussed this with OpenTTD upstream yesterday,
> > and they were already aware of the layout API removal and have been
> > casually looking at Harfbuzz and Pango as a replacement, but they do not
> > see an easy solution yet. ParagraphLayout seems to fit their usecase
> > quite neatly: they need internationalized word-wrapping of text (e.g.
> > also supporting right-to-left locales). Harfbuzz does not seem to offer
> > that, and Pango seems heavy-handed (and might not be easy to adapt to
> > OpenTTD's SDL renderer, and might not be portable enough).
> This is bad to read. I had the hope there's an easy solution and/or the
> replacement might be already in the making.
>
> > Neither me or upstream has much experience in this field, perhaps you
> > have a different suggestion for an alternative?
> I don't know any other alternative. Only OpenTTD uses the Paragraph Layout
> API and it makes me wonder what other solution the other projects use? I
> may think other games like Lincity-NG[3] also need internationalized text
> placement and/or LibreOffice still need to handle this as well. Do these
> have an alternative solution?

I'm currently dicussing these options with upstream, hopefull we can
find some alternative that works well and is maintained better.

Gr.

Matthijs


Bug#894159: OpenTTD, icu and ParagraphLayout

2018-04-29 Thread Matthijs Kooijman
Hi folks,

I'm the maintainer of the OpenTTD package, and stumbled on this bug
report, which refers to my package as the last user of the
ParagraphLayout API in icu.

First off: I have a pending new upstream version of OpenTTD that I'd
like to upload, but I don't want to interfere with this transition.
Should I hold it off, or do we expect that resolving these issues will
take a while and should I just upload it now (and have it build against
the current icu version)?


As for the layout issue, AFAIU the following is the case (but correct me
if I'm wrong):
 - ICU used to offer a layout API (which handles the layout of a single
   line of text).
 - Harfbuzz offers a similar layouting engine.
 - icu-le-hb is a separate piece of code that offers the (now removed)
   ICU layout API, by using Harfbuzz
 - ICU offers a ParagraphLayout API (which handles wordwrapping a piece
   of text). This code needs the (now removed) layout API, so currently
   it can only be built on top of icu-le-hb.

This gives a dependency chain where Harfbuzz optionally depends on ICU,
icu-le-hb depends on Harfbuzz and (I presume) ICU, and where
ParagraphLayout depends on icu-le-hb. Since ICU and ParagraphLayout live
in the same source package, this results in a circular dependency, which
needs two successive builds of the ICU package (once without
ParagraphLayout and once with), building icu-le-hb in between, to make
this work.

This does seem like a weird situation, also for ICU upstream. Do they
have any plans to resolve this?

One suggested solution (by László) is to integrate icu-le-hb into the
ICU source package, so the double compilation could at least happen
inside the ICU source package instead of having to be managed
externally. I do wonder: If icu-le-hb is properly integrated into the
ICU build system (probably needs integration upstream to be feasible),
this double build could be removed, right?

One alternative I can imagine is to move the ParagraphLayout code from
the ICU library (where it seems a bit out of place now) into the
icu-le-hb code. AFAICS that would resolve the circular dependency (or
does Harfbuzz need ICU and is that still a problem? That seems hard to
fix in any case...).


Another solution is of course to disable ParagraphLayout. László also
asked if OpenTTD, being the only user of this API, could migrate to
another solution. I've discussed this with OpenTTD upstream yesterday,
and they were already aware of the layout API removal and have been
casually looking at Harfbuzz and Pango as a replacement, but they do not
see an easy solution yet. ParagraphLayout seems to fit their usecase
quite neatly: they need internationalized word-wrapping of text (e.g.
also supporting right-to-left locales). Harfbuzz does not seem to offer
that, and Pango seems heavy-handed (and might not be easy to adapt to
OpenTTD's SDL renderer, and might not be portable enough).

Neither me or upstream has much experience in this field, perhaps you
have a different suggestion for an alternative?

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#755202: network-manager: keeps creating and using new connection "eth0" that does not work

2018-02-23 Thread Matthijs Kooijman
Hey folks,

previously, people suggested completely disabling ipv6 as a workaround
(or at least debugging tool). This, combined with the previous analysis
made me realize another workaround: Just disable ipv6 autoconf by the
kernel. I dropped these two lines in my sysctl.conf:

net.ipv6.conf.eth0.autoconf=0
net.ipv6.conf.eth0.accept_ra=0

Now, this bug seems to be gone (even with the 5-second sleep, tried
twice so far). From my logs, I see that a link-local ipv6 address is
still assigned, but apparently that does not trigger the broken
behaviour.

Also, this only prevents ipv6 configuration during the initial boot,
when the boot is complete, eth0 has a complete set of ipv6 addresses as
well (presumably because NetworkManager handles this).

Gr.

Matthijs


signature.asc
Description: PGP signature


Bug#755202: network-manager: keeps creating and using new connection "eth0" that does not work

2018-02-23 Thread Matthijs Kooijman
Package: network-manager
Version: 1.10.4-1+b1
Followup-For: Bug #755202

Hi folks,

I've also been seeing this bug on my system for quite some while now,
and decided to investigate today. I haven't quite figured it out, but
I'll share my findings here.

>From the previous discussion on this bug, most notably the IRC log
posted, I understand that the problem is somewhat like this:
 - On boot, something brings up the ethernet interface (or are network
   interfaces up by default?)
 - The interface gets a link-local ipv6 and/or statelessly
   autoconfigured global ipv6 address (handled by the kernel, I
   believe?).
 - NetworkManager starts, and finds a (partially configured interface).
 - NetworkManager starts managing the interface (with reason
   'connection-assumed') and creates a new (temporary) connection
   profile called 'eth0' that copies the current settings from the
   interface.
 - NetworkManager does *not* activate the normal default connection
   profile for the interface.

I haven't been able to confirm the above is exactly what happens, but my
observations do seem to match the above theory.



I have been trying to pinpoint the code path that handles it, without
much succes. I enabled trace logging (see NetworkManager.conf below) and
that caused debug and trace log entries to appear, but nothing I could
completely correlate with the source.

Here's the relevant bit of the log, where it apparently decides to
create a new connection. This is from 1.6.2, but with the output is
pretty much the same with 1.10.4.

  device (eth0): link connected
  manager: (eth0): new Ethernet device 
(/org/freedesktop/NetworkManager/Devices/2)
  manager: (eth0.10): new VLAN device 
(/org/freedesktop/NetworkManager/Devices/3)
  manager: (eth0.253): new VLAN device 
(/org/freedesktop/NetworkManager/Devices/4)
  keyfile: add connection in-memory 
(056caea2-6833-42cc-bdc5-bd8c5744c8a9,"eth0")
  device (eth0): state change: unmanaged -> unavailable (reason 
'connection-assumed') [10 20 41]
  device (eth0): state change: unavailable -> disconnected (reason 
'connection-assumed') [20 30 41]
  device (eth0): Activation: starting connection 'eth0' 
(056caea2-6833-42cc-bdc5-bd8c5744c8a9)

Looking at the source, I would expect these lines to come from:

  keyfile: add connection in-memory 
(056caea2-6833-42cc-bdc5-bd8c5744c8a9,"eth0")

https://github.com/NetworkManager/NetworkManager/blob/1.6.2/src/nm-manager.c#L1846

https://github.com/NetworkManager/NetworkManager/blob/1.6.2/src/nm-manager.c#L1762
  device (eth0): state change: unmanaged -> unavailable (reason 
'connection-assumed') [10 20 41]

https://github.com/NetworkManager/NetworkManager/blob/1.6.2/src/nm-manager.c#L1853-L1860
  device (eth0): state change: unavailable -> disconnected (reason 
'connection-assumed') [20 30 41]

https://github.com/NetworkManager/NetworkManager/blob/1.6.2/src/nm-manager.c#L1860

https://github.com/NetworkManager/NetworkManager/blob/1.6.2/src/nm-manager.c#L1793-L1797

However, in that case I would also expect to see these messages:

  
https://github.com/NetworkManager/NetworkManager/blob/1.6.2/src/nm-manager.c#L1758-L1760
  
https://github.com/NetworkManager/NetworkManager/blob/1.6.2/src/nm-manager.c#L1789-L1791

I've looked at other code paths that could lead to these
connection-assumed state changes, but each path I could find would also
log additional messages that I'm not seeing here.

It doesn't seem that debug messages for the device domain, or
nm-manager.c are broken, since I *do* see this message:

  manager: setup NMManager singleton (0x559e10330041)
  
https://github.com/NetworkManager/NetworkManager/blob/1.6.2/src/nm-manager.c#L5688

All this looks at 1.6.2, but I've had a similar (but not as thorough)
look at 1.10.4 which looks pretty much the same.



Another observation I made is that:
 - With 1.6.2, if I set two (out of two) VLAN connections I had to
   autostart=false, the problem would disappear.
 - With 1.10.4, if I set both VLAN connections to autostart=false, the
   problem would remain. Also if I completely removed them.
 - With 1.6.2, if I removed the VLAN connections, but added
   `ExecStartPre=/bin/sh -c "sleep 5"` to the systemd service file, the
   problem would appear again.

Given that, the link-up message from the kernel in my log is only
shortly before NM starts (within the same second), I suspect that there
is a race condition with NM looking at the interface and the kernel
configuring it, and the VLAN connections I have subtly influence the
timing.


I've also disabled avahi, which does not seem to influence the problem
at all (with avahi enabled, it generates log output about registering
addresses, but I believe it does this *in response to* the addresses
being added by the kernel, not the other way around.


-- System Information:
Debian Release: 9.3
  APT prefers stable
  APT policy: (990, 'stable'), (500, 'unstable-debug'), (500, 'testing-debug'), 
(500, 'stable-debug'), (500, 

Bug#875733: lxc.mount.auto = cgroup:mixed doesn't seem to work in Stretch anymore

2018-01-14 Thread Matthijs Kooijman
Package: lxc
Version: 1:2.0.7-2+deb9u1
Followup-For: Bug #875733

Hi folks,

I also ran into this exact issue. It seems upstream fixed this bug, see
https://github.com/lxc/lxc/issues/1737

I've backported this fix (along with some other commits it needs) to the
Debian stretch version, which works as expected. I've attached a patch
to the Debian packaging that does this. Since this is a regression from
earlier Debian versions, I guess this would be worth including in
stretch update?

One caveat to note: In my setup, I had `lxc.cgroup.use=@all` in my
`lxc.conf` file, which prevented this fix from working. See
https://github.com/lxc/lxc/issues/2084 for more details.

Gr.

Matthijs
From f1aa85a4b1c1c38211a9fa15afac72b3df142b3d Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matth...@stdin.nl>
Date: Sun, 14 Jan 2018 15:47:31 +0100
Subject: [PATCH] Backport upstream commits to fix running without
 CAP_SYS_ADMIN

Closes: #875733
---
 .../0011-lxc-cgroups-move-helper-functions.patch   | 166 ++
 .../0012-lxc-cgroups-handle-hubrid-layouts.patch   | 358 +
 .../0013-lxc-cgroups-without-cap-sys-admin.patch   | 157 +
 debian/patches/series  |   3 +
 4 files changed, 684 insertions(+)
 create mode 100644 debian/patches/0011-lxc-cgroups-move-helper-functions.patch
 create mode 100644 debian/patches/0012-lxc-cgroups-handle-hubrid-layouts.patch
 create mode 100644 debian/patches/0013-lxc-cgroups-without-cap-sys-admin.patch

diff --git a/debian/patches/0011-lxc-cgroups-move-helper-functions.patch 
b/debian/patches/0011-lxc-cgroups-move-helper-functions.patch
new file mode 100644
index 000..b1e7cea
--- /dev/null
+++ b/debian/patches/0011-lxc-cgroups-move-helper-functions.patch
@@ -0,0 +1,166 @@
+commit 04ad7ffe2a42fb2fa2e78e694990b385fd2dd5e0
+Author: Christian Brauner <christian.brau...@ubuntu.com>
+Date:   Wed Jul 26 14:57:35 2017 +0200
+
+utils: move helpers from cgfsng.c to utils.{c,h}
+
+Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
+
+---
+
+This commit was backported from upstream to Debian without changes, to
+make the patch 0013-lxc-cgroups-without-cap-sys-admin.patch apply.
+
+--- a/src/lxc/cgroups/cgfsng.c
 b/src/lxc/cgroups/cgfsng.c
+@@ -112,36 +112,12 @@ static void free_string_list(char **clis
+   }
+ }
+ 
+-/* Re-alllocate a pointer, do not fail */
+-static void *must_realloc(void *orig, size_t sz)
+-{
+-  void *ret;
+-
+-  do {
+-  ret = realloc(orig, sz);
+-  } while (!ret);
+-  return ret;
+-}
+-
+ /* Allocate a pointer, do not fail */
+ static void *must_alloc(size_t sz)
+ {
+   return must_realloc(NULL, sz);
+ }
+ 
+-/* return copy of string @entry;  do not fail. */
+-static char *must_copy_string(const char *entry)
+-{
+-  char *ret;
+-
+-  if (!entry)
+-  return NULL;
+-  do {
+-  ret = strdup(entry);
+-  } while (!ret);
+-  return ret;
+-}
+-
+ /*
+  * This is a special case - return a copy of @entry
+  * prepending 'name='.  I.e. turn systemd into name=systemd.
+@@ -253,8 +229,6 @@ struct hierarchy *get_hierarchy(const ch
+   return NULL;
+ }
+ 
+-static char *must_make_path(const char *first, ...) __attribute__((sentinel));
+-
+ #define BATCH_SIZE 50
+ static void batch_realloc(char **mem, size_t oldlen, size_t newlen)
+ {
+@@ -1165,33 +1139,6 @@ out_free:
+   return NULL;
+ }
+ 
+-/*
+- * Concatenate all passed-in strings into one path.  Do not fail.  If any 
piece is
+- * not prefixed with '/', add a '/'.
+- */
+-static char *must_make_path(const char *first, ...)
+-{
+-  va_list args;
+-  char *cur, *dest;
+-  size_t full_len = strlen(first);
+-
+-  dest = must_copy_string(first);
+-
+-  va_start(args, first);
+-  while ((cur = va_arg(args, char *)) != NULL) {
+-  full_len += strlen(cur);
+-  if (cur[0] != '/')
+-  full_len++;
+-  dest = must_realloc(dest, full_len + 1);
+-  if (cur[0] != '/')
+-  strcat(dest, "/");
+-  strcat(dest, cur);
+-  }
+-  va_end(args);
+-
+-  return dest;
+-}
+-
+ static int cgroup_rmdir(char *dirname)
+ {
+   struct dirent *direntp;
+--- a/src/lxc/utils.c
 b/src/lxc/utils.c
+@@ -2083,3 +2083,50 @@ int lxc_setgroups(int size, gid_t list[]
+ 
+   return 0;
+ }
++
++char *must_make_path(const char *first, ...)
++{
++  va_list args;
++  char *cur, *dest;
++  size_t full_len = strlen(first);
++
++  dest = must_copy_string(first);
++
++  va_start(args, first);
++  while ((cur = va_arg(args, char *)) != NULL) {
++  full_len += strlen(cur);
++  if (cur[0] != '/')
++  full_len++;
++  dest = must_realloc(dest, full_len + 1);
++  if (cur[0] != '/')
++  strcat(dest, "/");
++  strcat(des

Bug#840710: parted: Fix recognition of FAT file system after resizing

2017-10-28 Thread Matthijs Kooijman
Package: parted
Version: 3.2-17
Followup-For: Bug #840710

Hi,

I'd also be grateful if this patch could be backported. In the current
situation, resizing of bootable partitions simply is not possible
without breaking things.

I also tried downgrading to (lib)parted 3.1, but that caused segfaults
in fatresize, so that does not seem a feasible workaround either.

Gr.

Matthijs



Bug#875986: pam: Typo or pointless check in securetty patch

2017-09-16 Thread Matthijs Kooijman
Source: pam
Version: 1.1.8-3.6
Severity: normal

Hi maintainers,

When looking for some strange behaviour pam_unix, I came across a possible typo
in the securetty patch [1]. I'll highlight the relevant parts of the code
below.

if (isdigit(uttyname[0])) {
snprintf(ptname, sizeof(ptname), "pts/%s", uttyname);
} else {
ptname[0] = '\0';
}

This means that either:
 1) uttyname starts with a digit and ptname is "pts/$uttyname".
 2) utty name does not start with a digit and ptsname is "".

Then there is:

retval = 1;

while ((fgets(ttyfileline,sizeof(ttyfileline)-1, ttyfile) != NULL) 
   && retval) {
if(ttyfileline[strlen(ttyfileline) - 1] == '\n')
ttyfileline[strlen(ttyfileline) - 1] = '\0';
retval = ( strcmp(ttyfileline,uttyname)
   && (!ptname[0] || strcmp(ptname, uttyname)) );
}

And in particular, this part of the check:

(!ptname[0] || strcmp(ptname, uttyname))

In case 1) above, ptname is not empty and ptname can never equal uttyname, so
this resolves to (false || non-zero) == true. In case 2) above, this resolves
to (true || whatever) == true. Hence, I believe this part of the check just
means && true and could be omitted.

But more likely, there's a typo in this line and it should have read:

(!ptname[0] || strcmp(ptname, ttyfileline))

This would check each line from the securetty file against both uttyname and
ptname, instead of just against uttyname.

I'm not sure if the ptname part is actually needed at all, since this code
seems to work just fine without it for /dev/pts devices (but perhaps not in all
cases, I'm not sure what forms uttyname can have).

[1]: 
https://alioth.debian.org/scm/loggerhead/pkg-pam/debian/sid/view/head:/debian/patches-applied/054_pam_security_abstract_securetty_handling#L162

Gr.

Matthijs



Bug#785219: stterm: TERM environment var is incorrect

2017-09-14 Thread Matthijs Kooijman
Package: stterm
Followup-For: Bug #785219

Hi folks,

I can't really reproduce this problem, but for me `ls --color` outputs
color even when I run with TERM=foo or whatever, so there might be
something else going on.

However, I wanted to point out that the stterm package currently
provides stterm-256color termino (and some other variants), whereas the
ncurses-term package provides the st-256color termino (and again some
other variants).

Looking at xterm, that does not provide any terminfo at all, but relies
on the ncurses-term package to provide its terminfo (though it does not
Depend on it, so I guess it expects whoever needs terminfo to do so).
Splitting the terminfo off from the terminal itself makes it easier to
make things work remotely: When SSH'ing to a remote system, the remote
system needs the terminfo, but might not have the stterm available (or
even any GUI packages at all).

So, it seems this bug should be fixed by changing the TERM variable set,
and removing the terminfo from the stterm package.

Until then, placing this in your .bashrc should fix the problem in most
cases:

if [ "$TERM" = "stterm-256color" ]; then
TERM=st-256color;
fi

Gr.

Matthijs



Bug#873640: live-build: Hdd image fails due to hard links in binary directory

2017-09-04 Thread Matthijs Kooijman
Hi Raphael,

> Would you like to become co-maintainer of live-build? I maintain
> it because I use it in Kali but I use only a very limited set of
> the features and it would be nice to have other people who can
> look at bugs.
I won't really have time to dedicate to that, unfortunately. I'm happy
to keep providing patches for things I run into, of course. I'll also
subscribe to the bug reports, and try to pick up things when I have
time.

Gr.

Matthijs


signature.asc
Description: Digital signature


Bug#865586: live-build: binary_hdd failed with mkfs.vfat error

2017-08-29 Thread Matthijs Kooijman
Package: live-build
Version: 1:20170807
Followup-For: Bug #865586

Hi,

I also ran into this same error a while ago. It is caused by partition
devices being created when parted creates partitions, which are not
cleaned up when the loop devices are cleaned up. I'm attaching a patch
that fixes this, by passing --partscan to losetup to clear out any
lingering partition devices when the partition itself is mounted.

It would be more correct to clean up the partition devices when the loop
device is removed, but I'm not quite sure whose responsibility that
would be.

Gr.

Matthijs
>From 05e0703d82e0189e40d88f5070b1c2f955a0ad9e Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matth...@stdin.nl>
Date: Tue, 29 Aug 2017 15:04:31 +0200
Subject: [PATCH 3/4] Pass --partscan to losetup

Recent versions of Linux, parted or some other bit of software cause
partition devices, like /dev/loop0p1 to be created when running parted
mkpart. However, these devices are not cleaned up when running
losetup -d to remove /dev/loop0 later, so they linger around and confuse
mkfs (which refuses to make a filesystem, thinking there are partitions):

	mkfs.fat 4.1 (2017-01-24)
	mkfs.vfat: Partitions or virtual mappings on device '/dev/loop0', not making filesystem (use -I to override)

To prevent this behaviour, pass --partscan to losetup when adding a new
partition, to clean up any lingering partitions. It seems losetup does not
accept --partscan when deleting a loop device, to clean up at that point, but
since binary_hdd mounts the partition last, there should not be any lingering
partition devices after live-build is done.

The --partscan option is available since util-linux 2.21 (released in 2012), so
it should be fairly safe to pass it unconditionally.
---
 functions/losetup.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/functions/losetup.sh b/functions/losetup.sh
index 0346ff677..9d9b46e5d 100755
--- a/functions/losetup.sh
+++ b/functions/losetup.sh
@@ -40,7 +40,7 @@ Losetup ()
 	FILE="${2}"
 	PARTITION="${3:-1}"
 
-	${LB_LOSETUP} --read-only "${DEVICE}" "${FILE}"
+	${LB_LOSETUP} --read-only --partscan "${DEVICE}" "${FILE}"
 	FDISK_OUT="$(${LB_FDISK} -l -u ${DEVICE} 2>&1)"
 	Lodetach "${DEVICE}"
 
@@ -50,14 +50,14 @@ Losetup ()
 	then
 		Echo_message "Mounting %s with offset 0" "${DEVICE}"
 
-		${LB_LOSETUP} "${DEVICE}" "${FILE}"
+		${LB_LOSETUP} --partscan "${DEVICE}" "${FILE}"
 	else
 		SECTORS="$(echo "$FDISK_OUT" | sed -ne "s|^$LOOPDEVICE[ *]*\([0-9]*\).*|\1|p")"
 		OFFSET="$(expr ${SECTORS} '*' 512)"
 
 		Echo_message "Mounting %s with offset %s" "${DEVICE}" "${OFFSET}"
 
-		${LB_LOSETUP} -o "${OFFSET}" "${DEVICE}" "${FILE}"
+		${LB_LOSETUP} --partscan -o "${OFFSET}" "${DEVICE}" "${FILE}"
 	fi
 }
 
-- 
2.11.0



Bug#873640: live-build: Hdd image fails due to hard links in binary directory

2017-08-29 Thread Matthijs Kooijman
Package: live-build
Version: 1:20161202
Severity: normal
Tags: patch

Hi Raphaël,

building a hdd image currently fails. After fixing #865586 (mkfs
complaining about existing partitions) I see:

  $ sudo lb config -b hdd --bootloader syslinux
  $ sudo lb build
  (...)

  P: Copying binary contents into image...
  cp: error writing 'chroot/binary.tmp/live/initrd.img': No space left on device
  cp: error writing 'chroot/binary.tmp/live/initrd.img-4.9.0-3-amd64': No space 
left on device

To generate an hdd image, binary_hdd first estimates the needed size of
the image using du. By default, when du finds multiple hardlinked copies
of a file, it counts them only once. However, when the target filesystem
is FAT, which does not support hardlinks, these files will take up more
space when finally copying the contents.

To fix this I've attached a patch that, passes --count-links to du when
the target is FAT, to make the space estimation correct.

This problem is exposed by commit 9c974b26b (Instead of renaming kernel
for syslinux, create hardlinks), which might need to be separately fixed
(to not waste space on FAT targets), but binary_hdd should handle
hardlinks more gracefully in any case.

I've tested with version 1:20170807 and 1:20170829, but marked this bug
as found in 1:20161202 since that is the first version that has the
above commit.

Gr.

Matthijs
>From 0c313e9ed3d0de7cfa6ef8e8e21c01a08b8e4a8c Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matth...@stdin.nl>
Date: Tue, 29 Aug 2017 14:50:46 +0200
Subject: [PATCH 4/4] Handle hardlinks in binary_hdd

To generate an hdd image, binary_hdd first estimates the needed size of
the image using du. By default, when du finds multiple hardlinked copies
of a file, it counts them only once. However, when the target filesystem
is FAT, which does not support hardlinks, these files will take up more
space when finally copying the contents, breaking the build:

	P: Copying binary contents into image...
	cp: error writing 'chroot/binary.tmp/live/initrd.img-4.9.0-3-amd64': No space left on device
	cp: error writing 'chroot/binary.tmp/efi/boot/bootx64.efi': No space left on device
	cp: error writing 'chroot/binary.tmp/efi/boot/bootia32.efi': No space left on device
	cp: cannot create directory 'chroot/binary.tmp/boot/grub': No space left on device
	cp: cannot create directory 'chroot/binary.tmp/isolinux': No space left on device

To fix this, pass --count-links to du when the target is FAT, to make
the space estimation correct.

This problem is exposed by commit 9c974b26b (Instead of renaming kernel
for syslinux, create hardlinks), which might need to be separately fixed
(to not waste space on FAT targets), but binary_hdd should at least
handle hardlinks more gracefully.
---
 scripts/build/binary_hdd | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/scripts/build/binary_hdd b/scripts/build/binary_hdd
index 400403c0a..c6b842e95 100755
--- a/scripts/build/binary_hdd
+++ b/scripts/build/binary_hdd
@@ -97,6 +97,19 @@ then
 	rm -f ${LIVE_iMAGE_NAME}.img
 fi
 
+case "${LB_BINARY_FILESYSTEM}" in
+	fat*)
+		# If the target does not support hardlinks, tell du to
+		# count them double
+		DU_OPTIONS="--count-links"
+		;;
+
+	*)
+		DU_OPTIONS=""
+		;;
+esac
+
+
 # Enforce fat32 if we find individual files bigger than 2GB
 if [ "${LB_BINARY_FILESYSTEM}" = "fat16" ] && [ -n "$(find binary -size +1999M)" ]
 then
@@ -107,7 +120,7 @@ then
 fi
 
 # Enforce fat32 if we have images in total bigger than 2GB
-if [ "${LB_BINARY_FILESYSTEM}" = "fat16" ] && [ "$(du -s binary | awk '{ print $1 }')" -gt "190" ]
+if [ "${LB_BINARY_FILESYSTEM}" = "fat16" ] && [ "$(du ${DU_OPTIONS} -s binary | awk '{ print $1 }')" -gt "190" ]
 then
 	Echo_warning "FAT16 doesn't support partitions larger than 2GB, automatically enforcing FAT32"
 
@@ -127,7 +140,7 @@ fi
 # Everything which comes here needs to be cleaned up,
 if [ "$LB_HDD_SIZE" = "auto" ];
 then
-	DU_DIM="$(du -ms binary | cut -f1)"
+	DU_DIM="$(du ${DU_OPTIONS} -ms binary | cut -f1)"
 	REAL_DIM="$(Calculate_partition_size ${DU_DIM} ${LB_BINARY_FILESYSTEM})"
 else
 	REAL_DIM=$LB_HDD_SIZE
-- 
2.11.0



Bug#864629: live-build: When build hdd img: chroot/usr/lib/syslinux/mbr.bin’: No such file or directory

2017-08-28 Thread Matthijs Kooijman
Control: fixed -1 1:20161202
Control: notfixed 773833 5.0~a3-1
Control: fixed 773833 1:20161202

Hi,

it seems this bug wrt mbr.bin is fixed in live-build 1:20161202. The
changelog says:

  * Correct syslinux/extlinux mbr.bin path. Closes: #773833

And looking at the source, this is indeed the change needed to fix this
bug. This still leaves it broken in jessie (when combining the jessie
versions of live-build and syslinux), but it seems unlikely that this
fix will still be be backported now. I'll leave it up to the maintainer
to close this issue if needed, though.

Gr.

Matthijs


signature.asc
Description: Digital signature


Bug#873513: live-build: Running with LB_BUILD_WITH_CHROOT=false fails

2017-08-28 Thread Matthijs Kooijman
Package: live-build
Version: 1:20170807
Severity: normal
Tags: patch

Hi,

I've run into some problems running live-build with
LB_BUILD_WITH_CHROOT. I understand that this is not a recommended or
supported configuration (though I'm not entirely sure why not, given the
amount of code for it), but it's being used in the Webconverger project
since quite some time and it mostly works.

When running lb `lb config --build-with-chroot=false` followed by `lb
build`, the build fails with:

[2017-08-28 15:52:15] lb binary_syslinux
P: Begin installing syslinux...
E: /usr/share/ISOLINUX

It seems the check for ISOLINUX is using an old path, which is
corrected for chroot builds only.

I'm attaching a patch that unifies the dependency handling for with and
without chroot, which fixes this problem as well as adds some dependency
checks with LB_BUILD_WITH_CHROOT=false and simplifies some code. There
is a second patch (the first in the series), that makes sure that these
dependency checks actually give a proper error message for missing
packages on the host.

With these patches applied, the commands mentioned above produce a
working image.

Gr.

Matthijs

-- Package-specific info:

-- System Information:
Debian Release: stretch/sid
  APT prefers stable
  APT policy: (990, 'stable'), (500, 'testing'), (500, 'oldstable'), (50, 
'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.11.0-1-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages live-build depends on:
ii  debootstrap  1.0.87

Versions of packages live-build recommends:
ii  apt-utils   1.3.1
ii  cpio2.11+dfsg-5
ii  live-boot-doc   1:20160511
ii  live-config-doc 5.20160608
ii  live-manual-html [live-manual]  2:20151217.1
ii  wget1.18-4

live-build suggests no packages.

-- no debconf information
>From 406ed00e806abb84252033ac609fcc9bcd83 Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matth...@stdin.nl>
Date: Mon, 28 Aug 2017 15:14:34 +0200
Subject: [PATCH 1/2] Error out when needed packages are missing on the host

Previously, Check_package would only show an error when host packages
are missing on a non-apt system. On apt system, the packages would be
added to _LB_PACKAGES, which causes them to be installed in the chroot,
not in the host (or not at all if Install_package is not called). This
behaviour could break the build.

This applies to either packages that must be present in the host (as
checked with `Check_package host ...`), as well as packages that can be
either in the chroot or host (as checked with `Check_package chroot`)
when LB_BUILD_WITH_CHROOT=false.
---
 functions/packages.sh | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/functions/packages.sh b/functions/packages.sh
index c2f7cfabf..7c3592dac 100755
--- a/functions/packages.sh
+++ b/functions/packages.sh
@@ -16,16 +16,16 @@ Check_package ()
 
 	Check_installed "${CHROOT}" "${FILE}" "${PACKAGE}"
 
-	case "${INSTALL_STATUS}" in
-		1)
+	if [ "${INSTALL_STATUS}" -ne 0 ]
+	then
+		if [ "${LB_BUILD_WITH_CHROOT}" != "false" ]
+		then
 			_LB_PACKAGES="${_LB_PACKAGES} ${PACKAGE}"
-			;;
-
-		2)
+		else
 			Echo_error "You need to install %s on your host system." "${PACKAGE}"
 			exit 1
-			;;
-	esac
+		fi
+	fi
 }
 
 Install_package ()
-- 
2.11.0

>From 034bb0c00788f0927a068d151c53a8f341e215fa Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matth...@stdin.nl>
Date: Mon, 28 Aug 2017 11:29:54 +0200
Subject: [PATCH 2/2] Check all dependencies independent of
 LB_BUILD_WITH_CHROOT

Since commit fdc9250bc (Changing package dependency checks within chroot
to work outside as well), Check_package automatically checks for
LB_BUILD_WITH_CHROOT and works inside as well as outside of the chroot,
so no need to check LB_BUILD_WITH_CHROOT before calling them.
Install_package and Remove_package are just a no-op when building
without chroot, so they can also be called unconditionally.
Restore_cache and Save_cache do not check LB_BUILD_WITH_CHROOT but it
it should not hurt to call them when not needed (which already happened
in some cases).

This commit makes all Check_package calls unconditional on
LB_BUILD_WITH_CHROOT.

For binary_syslinux, this fixes the check (which used outdated paths
outside the chroot since 7b6dfd9d1), for binary_grub-efi,
binary_package-lists and chroot_package-lists this simplifies the code
(but also causes the check to become package-based instead of file-based
on apt-based systems), and for binary_loadlin and binary_win32-loader
this adds the check outside the chroot which was previously missing.
---
 scripts/build/binary_grub-efi  

Bug#868377: ofono: Consider packaging test scripts

2017-07-15 Thread Matthijs Kooijman
Source: ofono
Severity: wishlist

Hi,

The oFono git repository contains a number of helper scripts in the
test/ directory. These python scripts help to query the internal state of oFono
through dbus calls, and it would be helpful to have them available. It
is probably best to put them in a separate package, since they depend on
python3 and the python3-dbus package.

Gr.

Matthijs



Bug#859439: git-buildpackage: When merge fails due to untracked files, subsequent rollback also fails

2017-04-03 Thread Matthijs Kooijman
Hi Guido,

> The rollback overall doesn't fail, only the part that tries to undo the
> merge. Rollbacks are always best effort (roll back as much as you can)
> and so master, etc. end up on the correct revs.  However we must not
> fail the merge rollback.
Yeah, exactly.

> > Looking more closely at the original merge failure, it aborts because
> > there are untracked files that would be overwritten by the merge (which
> > likely means it never starts the merge, instead of starting and failing
> > later). Perhaps git merge returns different exit codes for these
> > situations?
> 
> Sadly not but we can look for MERGE_HEAD. I've checked in a patch for
> that and it would be awesome if you could test it (or tell me which
> files it stumbled upon in openttd)?
I don't have a gbp build set up, so if you could test it, that would be
great. I haven't pushed the 1.7.0 changes to alioth yet, so you should
be able to test with:

$ gbp clone https://anonscm.debian.org/git/collab-maint/openttd.git
$ cd openttd
$ touch bin/baseset/orig_extra.grf
$ gbp import-orig --uscan --verbose

(not tested)

Gr.

Matthijs


signature.asc
Description: Digital signature


Bug#859439: git-buildpackage: When merge fails due to untracked files, subsequent rollback also fails

2017-04-03 Thread Matthijs Kooijman
Package: git-buildpackage
Version: 0.8.12.2
Severity: normal

I just ran gbp import-orig to update a package, and somehow the merge
fails. The subsequent rollback also reports an error, but looking at the
repository state the master, upstream and pristine-tar branches are
unmodified (probably because the merge to master never happened, so
there is no need to abort it?).

Looking more closely at the original merge failure, it aborts because
there are untracked files that would be overwritten by the merge (which
likely means it never starts the merge, instead of starting and failing
later). Perhaps git merge returns different exit codes for these
situations?

Gr.

Matthijs

$ gbp import-orig --uscan --verbose
gbp:warning: Old style config section [git-import-orig] found please rename to 
[import-orig]
gbp:debug: ['git', 'rev-parse', '--show-cdup']
gbp:debug: ['git', 'rev-parse', '--is-bare-repository']
gbp:debug: ['git', 'rev-parse', '--git-dir']
gbp:debug: ['git', 'for-each-ref', '--format=%(refname:short)', 'refs/heads/']
gbp:debug: ['git', 'show-ref', 'refs/heads/upstream']
gbp:debug: ['git', 'status', '--porcelain']
gbp:info: Launching uscan...
uscan: Newest version of openttd on remote site is 1.7.0, local version is 1.6.1
uscan:=> Newer package available from
  http://binaries.openttd.org/releases/1.7.0/openttd-1.7.0-source.tar.xz
uscan warn: Skipping ./os/debian/watch
   as this package has already been scanned successfully
gbp:info: using ../openttd_1.7.0.orig.tar.xz
What is the upstream version? [1.7.0]
gbp:debug: ['git', 'tag', '-l', 'upstream/1.7.0']
gbp:debug: tar ['-C', '../tmpaBKUpQ', '-a', '-xf', 
'../openttd_1.7.0.orig.tar.xz'] []
gbp:debug: Unpacked '../openttd_1.7.0.orig.tar.xz' to 
'../tmpaBKUpQ/openttd-1.7.0'
gbp:info: Importing '../openttd_1.7.0.orig.tar.xz' to branch 'upstream'...
gbp:info: Source package is openttd
gbp:info: Upstream version is 1.7.0
gbp:debug: ['git', 'show-ref', 'refs/heads/upstream']
gbp:debug: ['git', 'rev-parse', '--quiet', '--verify', 'upstream']
gbp:debug: ['git', 'add', '-f', '.']
gbp:debug: ['git', 'write-tree']
gbp:debug: ['git', 'rev-parse', '--quiet', '--verify', 'upstream']
gbp:debug: ['git', 'commit-tree', '79ac9215a213d3a3e8b76957dfb68375804bdabb', 
'-p', '8cb8348b46ec8ccb0ad236f6815504e3a392a625']
gbp:debug: ['git', 'update-ref', '-m', 'gbp: New upstream release 1.7.0.', 
'refs/heads/upstream', '3fbd15d80f1f282a83c1bd700483a96148dbc317', 
'8cb8348b46ec8ccb0ad236f6815504e3a392a625']
gbp:debug: ['git', 'show-ref', 'refs/heads/pristine-tar']
gbp:debug: ['git', 'rev-parse', '--quiet', '--verify', 'pristine-tar']
gbp:debug: ['git', 'ls-tree', '-z', 'upstream', '--']
gbp:debug: ['git', 'mktree', '-z']
gbp:debug: /usr/bin/pristine-tar [] ['commit', '../openttd_1.7.0.orig.tar.xz', 
'79ac9215a213d3a3e8b76957dfb68375804bdabb']
gbp:debug: ['git', 'tag', '-m', 'Upstream version 1.7.0', '-s', '-u', 
'A1565658', 'upstream/1.7.0', '3fbd15d80f1f282a83c1bd700483a96148dbc317']
gbp:debug: ['git', 'show-ref', 'refs/heads/master']
gbp:debug: ['git', 'rev-parse', '--quiet', '--verify', 'master']
gbp:info: Merging to 'master'
gbp:debug: ['git', 'symbolic-ref', 'HEAD']
gbp:debug: ['git', 'show-ref', 'refs/heads/master']
gbp:debug: ['git', 'symbolic-ref', 'HEAD']
gbp:debug: ['git', 'show-ref', 'refs/heads/master']
gbp:debug: ['git', 'help', 'merge', '-m']
gbp:debug: ['git', 'merge', '--no-summary', '--no-edit', 'upstream/1.7.0']
gbp:error: Automatic merge failed.
gbp:error: Error detected, Will roll back changes.
gbp:info: Rolling back branch upstream by resetting it to 
8cb8348b46ec8ccb0ad236f6815504e3a392a625
gbp:debug: ['git', 'update-ref', '-m', 'gbp import-orig: failure rollback of 
upstream', 'refs/heads/upstream', '8cb8348b46ec8ccb0ad236f6815504e3a392a625']
gbp:info: Rolling back branch pristine-tar by resetting it to 
9f75dd82a683b03662ca736f68f415e6f7fa6dfe
gbp:debug: ['git', 'update-ref', '-m', 'gbp import-orig: failure rollback of 
pristine-tar', 'refs/heads/pristine-tar', 
'9f75dd82a683b03662ca736f68f415e6f7fa6dfe']
gbp:info: Rolling back tag upstream/1.7.0 by deleting it
gbp:debug: ['git', 'tag', '-l', 'upstream/1.7.0']
gbp:debug: ['git', 'tag', '-d', 'upstream/1.7.0']
gbp:info: Rolling back branch master by resetting it to 
704027decfff0924ffb363c40924cff947f38750
gbp:debug: ['git', 'update-ref', '-m', 'gbp import-orig: failure rollback of 
master', 'refs/heads/master', '704027decfff0924ffb363c40924cff947f38750']
gbp:info: Rolling back failed merge of upstream/1.7.0
gbp:debug: ['git', 'merge', '--abort']
gbp:error: Automatic rollback failed [('upstream/1.7.0', 'commit', 
'abortmerge', None, GitRepositoryError('Error running git merge: fatal: There 
is no merge to abort (MERGE_HEAD missing).\n',))]
gbp:error: Clean up manually and please report a bug: [('upstream/1.7.0', 
'commit', 'abortmerge', None, GitRepositoryError('Error running git merge: 
fatal: There is no merge to abort (MERGE_HEAD missing).\n',))]
gbp:debug: rm ['-rf', '../tmpaBKUpQ'] []




-- 

Bug#825681: openttd: Please add StrategyGame category to desktop file

2016-05-29 Thread Matthijs Kooijman
Hi Peter,

> This game is part of the games-strategy meta package, but do not show up
> under the strategy subsection of the Game KDE menu.  The reason is that
> the desktop file is missing the 'StrategyGame' category.  Please add it
> to the desktop file.
Good suggestion. I'll change this with the next upstream version.

Thanks,

Matthijs


signature.asc
Description: Digital signature


  1   2   3   4   5   >