Bug#895061: exclusions and dh_missing

[Niels Thykier]

On Thu, 22 Aug 2019 11:56:34 +0200 Nicolas Boulenguez
 wrote:
> Source: debhelper
> Followup-For: Bug #895061
> 
> #935309 (virtualbox) is already closed.
> 
> #877267 (ocaml) is closed in experimental/4.08.0-3 but not yet in unstable
> On the other hand, it uses compatibility level 12.
> 
> It seems possible to drop --ignore in compatibility 13.
> 
> 

Hi,

I have just applied a commit to remove --ignore now that the remaining
consumers have stopped using --ignore in unstable and testing.

Thanks,
~Niels

Bug#895061: exclusions and dh_missing

[Nicolas Boulenguez]

Source: debhelper
Followup-For: Bug #895061

#935309 (virtualbox) is already closed.

#877267 (ocaml) is closed in experimental/4.08.0-3 but not yet in unstable
On the other hand, it uses compatibility level 12.

It seems possible to drop --ignore in compatibility 13.

Bug#895061: exclusions and dh_missing

[Nicolas Boulenguez]

Source: debhelper
Followup-For: Bug #895061

Hello.

The 'ocaml' maintainers have closed #877267, but a new search shows
that 'virtualbox' also uses --ignore. I have reported #935309, with a
trivial patch getting the same effect by other means.

Once #935309 is fixed, the attachment removes support for --ignore
(it replaces the previous one).
--- a/debhelper.pod
+++ b/debhelper.pod
@@ -185,13 +185,10 @@
 
 =item B<--ignore=>I
 
-Ignore the specified file. This can be used if F contains a debhelper
-config file that a debhelper command should not act on. Note that
-F, F, and F can't be ignored, but
-then, there should never be a reason to ignore those files.
-
-For example, if upstream ships a F that you don't want
-B to install, use B<--ignore=debian/init>
+The removal of this option is planned in compat 13.  If you cannot
+acheive your goal with L or executable debhelper
+configuration files, please describe your needs at
+L.
 
 =item B<-P>I, B<--tmpdir=>I
 
@@ -962,6 +959,10 @@
 If you need them for B<*-indep> targets, you can add an explicit
 Build-Depends on B.
 
+*item -
+
+The B<--ignore> command line option is deprecated.
+
 =back
 
 =back
--- a/lib/Debian/Debhelper/Dh_Getopt.pm
+++ b/lib/Debian/Debhelper/Dh_Getopt.pm
@@ -79,6 +79,7 @@
 
 # Add a file to the ignore list.
 sub AddIgnore { my($option,$file)=@_;
+	compat(12) || error("compatibility level 13 deprecates --ignore");
 	$dh{IGNORE}->{$file}=1;
 }
 
--- a/lib/Debian/Debhelper/Dh_Lib.pm
+++ b/lib/Debian/Debhelper/Dh_Lib.pm
@@ -978,6 +978,7 @@
 		}
 		foreach my $file (@try) {
 			if (-f $file &&
+# dh{IGNORE} may only exist in compat(12)
 (! $dh{IGNORE} || ! exists $dh{IGNORE}->{$file})) {
 return $file;
 			}

Bug#895061: exclusions and dh_missing

[Nicolas Boulenguez]

> Ok, just to confirm [...] ?
Precisely.


The following tools only apply -X in case 1. This is useful.
  dh_autoreconf dh_fixperms dh_installchangelogs dh_md5sums dh_prep
  dh_shlibdeps dh_strip dh_dwz dh_makeshlibs dh_link

The following tools apply -X to both case 1 and 2, but 2 is never used
in practice.I suggest dropping support for case 2.

  dh_clean\s.*(?:-X|--exclude)) shows 138 results.
  All of them excluding paths matching the default pattern, with no or
  unrelated command line arguments.
  The implementation applies -X to config file, but not to command
  line arguments, and no none seems to have noticed.
  Job already half done :-).

  dh_compress seems similar, though I have not checked each one of the
  1536 results.


Only 3 tools frequently use --exclude to refine command-line or
config-file patterns.
  dh_install (and its old variant dh_movefiles, 1 result)
  dh_installdocs
  dh_installexamples

> 2) The case you described above where people want "all the files in
>foo except bar".

After reading many examples, I realize how this can be convenient
("dh_installexamples -X.cvsignore").


> 1) Ignore files/paths from the config file that are conditionally
>built.  This is usually observed via the following pattern:
> [...]

In my opinion, such a construct is an abuse of -X. It is better to
* remove foo from debian/pkg.install (tell the truth)
* add foo to debian/not-installed(tell the truth)
* use a positive selection:  (each negation hinders readability)
  PKG_INSTALL :=
  ifeq ...
PKG_INSTALL += foo
  endif
  [...]
dh_install --package=pkg $(PKG_INSTALL)
dh_install --remaining-packages
The difference is cosmetic for the maintainer, but not in favor of -X.


> [executable config files and dh-exec]

If dh-exec finds a new maintainer, it may be the best option to
implement exclusions outside debhelper (#498067 has a pending patch).


> If you can convince the remaining consumers of --ignore to migrate
> away, then I am happy to drop the option when we release debhelper 12.

This seems possible:

* gutenprint: reported as #895659.

* ocaml: patches #877267 as a side effect.

* lintian: in t/tests/debhelper-dh-exec/
  
https://salsa.debian.org/lintian/lintian/commit/7ed7a25acf523d2a820785841be96a01814c5f2f
 If I understand well, it should be sufficient to
 * rename debian/debian/docs to debian/debian/mime.
 * replace lines 6-9 of debian/debian/rules with "override_dh_installmime:"
 * replace "debian/docs" with "debian/mime" at end of line 8 in the tags file.
 As the author of the commit, you should be able to answer :-)

Bug#895061: exclusions and dh_missing

[Niels Thykier]

Nicolas Boulenguez:
> My understanding (so far) is that there are three unselection
> mechanisms, and I have the feeling that only one is worth supporting.
> 

Ok, just to confirm, I understood you correct, those three are:

 1) --exclude for paths debhelper automatically work on (but not
--ignore)

 2) --exclude for paths provided by the maintainer either via arguments
or config file.

 3) --ignore

And 1) is the only one you want to preserve?

> 
> [...]
> 
> The --exclude option has a second, distinct, and arguable usage:
> to extend the expressivity of glob patterns, like in:
>   override_dh_installexamples:
> dh_installexamples --exclude=foo/bar foo
> 

I know of two common use-cases here for --exclude.  If we can find an
alternative for those, I am happy to consider deprecating --exclude for
these purposes.

  1)  Ignore files/paths from the config file that are conditionally
  built.  This is usually observed via the following pattern:

  IGNORE_FOO :=
  ifeq ...
  IGNORE_FOO := -X foo
  endif

  [...]
dh_install $(IGNORE_FOO)

  2) The case you described above where people want "all the files in
 foo except bar".

You later mention the executable config files as a possible solution.  I
disagree that executable config files /in their current form/ is ready
to replace these patterns.
  That said,  I know dh-exec is doing a good job at making executable
config files easier to work with.  It is sadly missing someone with
interest and motivation to maintain it and I have been hoping someone
picked it up (I do not have a vision for dh-exec, so I am not the right
one to pick it up).

Note that I am very happy to work with you (or anyone else) on improving
dh-exec and the debhelper <=> dh-exec integration.  If the changes are
useful and stable, we can look at merging some of them back eventually
(dh-exec has a much better interface for prototyping, while debhelper
effectively commits to 10-12 years of support the moment it is released
in a compat level).

> [...]
> 
> It forces debhelper to be less performant, as each existing path must
> be matched against two distinct kinds of patterns in the end.
> Also, --exclude prevents a simple 'chmod -a' for directories.
> 

While I loath the fact that --exclude makes things hard to optimize, I
am not entirely convinced the overhead of using --exclude is an issue in
itself at the moment.

Performance-wise, I suspect #866581 is a much lower hanging fruit.  Or
the fact that Dh_Lib takes 0.035s to load costing us 100ms per 3
debhelper command run during build on pure "do the same initialization
over and over again".

> It forces debhelper to contain code that is hard to write, maintain
> and test.

I admit that most --exclude is hard to test.  The notable exception
being "excludefile", which is rather easy to test (compared to things in
debhelper in general).

> Factorization like EXCLUDE_FIND is only a work-around, it
> does not change that a Perl script generates a Shell command
> describing a find request compiling regular expressions translated
> from command-line arguments usually transmited by a Shell launched by
> Make. Who would bet that all special characters are handled correctly
> in all corner cases ?
> 

EXCLUDE_FIND will correctly be passed to find, but every thing else is
indeed a hassle to quote correctly.  Related annoyances: #864182

I would indeed prefer less need for "complex_doit".

> [...]
> 
> To some extent, the same concern applies to --ignore.
> [...]
> Is it worth to slow down each call to pkgfile() to handle this?
> However, the performance and complexity costs are way smaller, so the
> balance with the burden of removing an option is probably different.
> 

I was going argue that it was not worth the removal. But there is only
one or two consumers (left?) of --ignore according to codesearch[1].
Even cdbs does not appear to use it at all.
  If you can convince the remaining consumers of --ignore to migrate
away, then I am happy to drop the option when we release debhelper 12.

Thanks,
~Niels

[1] https://codesearch.debian.net/search?q=dh.*.%5Cs--ignore

E.g. gutenprint

Bug#895061: exclusions and dh_missing

[Nicolas Boulenguez]

My understanding (so far) is that there are three unselection
mechanisms, and I have the feeling that only one is worth supporting.


It is useful to provide at least a sensible way to overrides hardcoded
or autodetected paths.
  override_dh_link:
dh_link --exclude=/non/standard/symlink
  override_dh_compress:
dh_compress --exclude=/non/standard/format


The --exclude option has a second, distinct, and arguable usage:
to extend the expressivity of glob patterns, like in:
  override_dh_installexamples:
dh_installexamples --exclude=foo/bar foo

It allows the caller to express two contradictory wishes, leading
debhelper to guess the actual intention (should dh_missing complain?).

It mixes two kinds of matching patterns in an inconsistent interface.
* dh_installexamples 'foo*' --exclude=foo
  seems to install foobar but does not.
* dh_installexamples foo --exclude=foo/bar
  seems to install foo/foo/bar but does not.
Good documentation is only a work-around.

For more fun, the incompatible wishes may be in different locations:
'foo*' in debian/examples and '--exclude=foo' in debian/rules.

It forces debhelper to be less performant, as each existing path must
be matched against two distinct kinds of patterns in the end.
Also, --exclude prevents a simple 'chmod -a' for directories.

It forces debhelper to contain code that is hard to write, maintain
and test. Factorization like EXCLUDE_FIND is only a work-around, it
does not change that a Perl script generates a Shell command
describing a find request compiling regular expressions translated
from command-line arguments usually transmited by a Shell launched by
Make. Who would bet that all special characters are handled correctly
in all corner cases ?

Now consider that all this can be replaced with an explicit debian/examples
  foo/baz
  foobar
or if the list keeps varying by an executable debian/examples with
  #!/bin/sh
  find foo -mindepth 1 -maxdepth 1 -not -name bar
(and of course foo/bar in debian/not-installed).


To some extent, the same concern applies to --ignore.
  override_dh_foo:
dh_foo --package=pkg --ignore=debian/pkg.cfg
dh_foo --remaining-packages
The now common source format 3.0 has made the original motivation
obsolete (if upstream tarball contains a debian/ subdirectory, it is
not extracted by dpkg-source at all).
If necessary, the maintainer can remove debian/pkg.cfg, rename it, or
add it to debian/clean.
Is it worth to slow down each call to pkgfile() to handle this?
However, the performance and complexity costs are way smaller, so the
balance with the burden of removing an option is probably different.

Bug#895061: exclusions and dh_missing

[Niels Thykier]

Nicolas Boulenguez:
> Package: debhelper
> Version: 11.1.6
> Severity: wishlist
> 
> Hi.
> 

Hi, :)

> Some debhelper commands log some files as installed while they are not,
> so that dh_missing does not report them later.
> * files missing because of nodocs
> * files ignored because the package is not selected (-p/-N/-a/-i or similar)
> 
> Should excluded files also be marked as installed?
> (for example, dh_installexamples does so)

To clarify the "true" purpose of the "installed-by" logs:

They are intended to make the integration between dh_install* and
dh_missing "just work(tm)" for people (in the average case).

This is complicated by things like builds with dpkg-buildpackage
-A/-B/-b or builds with various profiles enabled, where the set of
packages are not always the same.  Not to mention that sometimes people
call the same helper multiple times with only some arguments and expect
everything to work, etc.

What should be logged?  The files and folders necessary for dh_missing
to not incorrectly complain when you use dh_missing
--(list|fail)-missing and build with any variation of "dpkg-buildpackage
-A/-B/-b plus build profiles".

I have ignored --exclude and just logged files:

 1) It is easier + faster
 2) If people have excluded it, they have actively considered the file
and probably know what they are doing.

It is true that they could have have excluded it because they wanted to
install it in a separate call but forgot.  So far that has not been a
problem.  Note that one of things that people use dh_missing for is to
detect when upstream provides new files or renamed something.

> The selected behaviour should probably be explicit in the --exclude
> section of debhelper(7).
> 

Certainly.

> Please allow me to suggest a third way: reserve --exclude for
> overrides of hardcoded lists, but ignore this option for explicit user
> selections.
> 

Note that if you by "user selection" mean e.g. command line arguments or
patterns in the config file, then that is exactly what --exclude was for.

Whereas --ignore is useful for ignoring a "config file" in debian.


> For example, --exclude is useful to prevent dh_compress acting on a
> specific file, or dh_installdocs acting on a specific
> debian/changelog,

Note: The dh_installdocs example should have been dh_installchangelogs
and should be using --ignore and not --exclude.  (And in the concrete
case, --ignore cannot used for that file as dh_installchangelogs insists
on having a Debian changelog).

> but "dh_installexamples --exclude=foo/bar foo" can
> be avoided with a more specific pattern.
> 

I think I might need a bit more here.  I do not see what the problem is
with using "--exclude foo/bar foo".  You could have:

  foo/bar/...
  foo/baz
  foo/baz/bar
  foo/baz/baz
  foo/foobar
  foo/...

In that case, --exclude foo/bar would only exclude the first item (i.e.
the directory and everything beneath it) but not foo/baz/bar.

> Even if the caller ends up generating a list, the result will probably
> be more maintainable that black magic inside the dh_ tool.
> See dh_installexamples for an example of the complexity added by --exclude.
> 

The complexity of dh_installexamples (and similar commands) is related
to the ability to exclude folders while still being somewhat efficient
at copying over the files (doing a copy per file is expensive).
  Some of that could be handled by refactoring the (almost) same code in
the helpers using that pattern and move it into Dh_Lib.

Thanks,
~Niels

Bug#895061: exclusions and dh_missing

[Nicolas Boulenguez]

Package: debhelper
Version: 11.1.6
Severity: wishlist

Hi.

Some debhelper commands log some files as installed while they are not,
so that dh_missing does not report them later.
* files missing because of nodocs
* files ignored because the package is not selected (-p/-N/-a/-i or similar)

Should excluded files also be marked as installed?
(for example, dh_installexamples does so)
The selected behaviour should probably be explicit in the --exclude
section of debhelper(7).

Please allow me to suggest a third way: reserve --exclude for
overrides of hardcoded lists, but ignore this option for explicit user
selections.

For example, --exclude is useful to prevent dh_compress acting on a
specific file, or dh_installdocs acting on a specific
debian/changelog, but "dh_installexamples --exclude=foo/bar foo" can
be avoided with a more specific pattern.

Even if the caller ends up generating a list, the result will probably
be more maintainable that black magic inside the dh_ tool.
See dh_installexamples for an example of the complexity added by --exclude.