Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.bugs.dist > #1092709 > unrolled thread

Bug#872381: dpkg-dev: optimize Makefile snippets for debian/rules

Back to article view | Back to linux.debian.bugs.dist

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Bug#872381: dpkg-dev: optimize Makefile snippets for debian/rules Nicolas Boulenguez <nicolas@debian.org> - 2022-02-10 22:00 +0100
    Bug#872381: dpkg-dev: optimize Makefile snippets for debian/rules Guillem Jover <guillem@debian.org> - 2022-02-10 23:40 +0100
      Bug#872381: dpkg-dev: optimize Makefile snippets for debian/rules Nicolas Boulenguez <nicolas@debian.org> - 2022-02-11 20:30 +0100

#1092709 — Bug#872381: dpkg-dev: optimize Makefile snippets for debian/rules

Package: dpkg-dev
Followup-For: Bug #872381

(CCing Guillem as the author of the commits referenced below)

Hello.

The changes introduced by 1947fef8 and 981d5acb are basically
incompatible with the first patch in bug report #872381 [1].

For the record, commits a49f0a9c and f28cbcc4 also require a rebase of
the patches from #872381. The merge is trivial, if not automatic for
git.

The --in-place sed option ([1]) may simplify scripts/mk/Makefile.am
much more than the do_make_subst variable provided by
build-aux/subst.am does. It is for now the only Makefile using
do_make_subst.

On the other hand, do_make_subst (from 1947fef8 and 981d5acb) is more
flexible and allows the source and generated paths to differ if ever
necessary.

If you select this option, I suggest to rename
scripts/mk/{default,buildtools}.mk to scripts/mk/*.mk.sed or similar
(for example, .sed.mk in order to keep syntax highlighting).  Distinct
source and object files would also simplify scripts/mk/Makefile a lot.

More generally, I would really appreciate the opinion of a dpkg
maintainer about the changes suggested in
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=872381.  Rebasing
and testing them again and again is an ungrateful work.  Even "these
changes seem interesting but require a careful review that we cannot
afford now" would be more motivating than no answer at all.

Thank you for your work on dpkg.

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=872381;filename=0001-scripts-mk-simplify-Makefile.am-with-sed-in-place-op.patch;msg=30

[toc] | [next] | [standalone]

#1092720

Hi!

On Thu, 2022-02-10 at 21:53:59 +0100, Nicolas Boulenguez wrote:
> Package: dpkg-dev
> Followup-For: Bug #872381

> The --in-place sed option ([1]) may simplify scripts/mk/Makefile.am
> much more than the do_make_subst variable provided by
> build-aux/subst.am does. It is for now the only Makefile using
> do_make_subst.

I'm aware of -i, the problem is that this is unfortunately not portable,
see the comment f.ex. in scripts/Makefile.am, should perhaps add a
similar one in the new subst.am. Will also probably fix the few
instances in the test suites.

> If you select this option, I suggest to rename
> scripts/mk/{default,buildtools}.mk to scripts/mk/*.mk.sed or similar
> (for example, .sed.mk in order to keep syntax highlighting).  Distinct
> source and object files would also simplify scripts/mk/Makefile a lot.

Yeah, that would be more convenient, the problem is that these files
need to have specific pathnames during tests, which are different from
the ones at run-time. Those could be set from the environment, but that
would miss the case where the variable is completely missing from the
file. :)

> More generally, I would really appreciate the opinion of a dpkg
> maintainer about the changes suggested in
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=872381.  Rebasing
> and testing them again and again is an ungrateful work.  Even "these
> changes seem interesting but require a careful review that we cannot
> afford now" would be more motivating than no answer at all.

Right, sorry, I keep seeing the mails, and concoct the replies in my
head, and either then think I've already replied or miss doing that
entirely. :/

The main issue I see is that this trades performance depending on the
mode of operation. Some of these variables are (currently) not
expected to be set by the driver calling debian/rules (in Debian we
still have to support that being any thing :/), but then most of our
tools do go through dpkg-buildpackage. So the various «.mk» cannot
expect these variables to be previously set, but should assume that
the common case is them being executed through dpkg-buildpackage, and
that one setting some of those, which means unconditionally setting
some of them will make the package builds somewhat slower. The other
variables should then not be set if not used, as that would slow down
packages that do not need them.

Ideally, most of this would not be needed at all, because we could
rely on dpkg-buildpackage having exported all of these, which it
already needs to compute in most cases anyway, but this is something
we cannot have in Debian for now. I'm pondering adding a mode, not to
be used by Debian, which could be used by local packages or other
distros though.

In any case, thanks for the work here, and sorry for the repeated
rebasing. I'll go over these in the coming days.

Thanks,
Guillem

[toc] | [prev] | [next] | [standalone]

#1092817

> > [in scripts/mk/Makefile.am], I suggest to rename
> > scripts/mk/{default,buildtools}.mk to scripts/mk/*.mk.sed or similar
> > (for example, .sed.mk in order to keep syntax highlighting).  Distinct
> > source and object files would also simplify scripts/mk/Makefile a lot.

> Yeah, that would be more convenient, the problem is that these files
> need to have specific pathnames during tests, which are different from
> the ones at run-time. Those could be set from the environment, but that
> would miss the case where the variable is completely missing from the
> file. :)

Would this be OK?

dpkg_datadir := $(shell $(dir $(lastword $(MAKEFILE_LIST))))

If I remember well, only GNU Make supports this, but else something
like
dpkg_datadir != dirname `echo $(MAKEFILE_LIST) | sed 's/.* //'`
could probably be improved to compute the result only once.

> The main issue I see is that this trades performance depending on the
> mode of operation. Some of these variables are (currently) not
> expected to be set by the driver calling debian/rules (in Debian we
> still have to support that being any thing :/), but then most of our
> tools do go through dpkg-buildpackage. So the various «.mk» cannot
> expect these variables to be previously set, but should assume that
> the common case is them being executed through dpkg-buildpackage, and
> that one setting some of those, which means unconditionally setting
> some of them will make the package builds somewhat slower. The other
> variables should then not be set if not used, as that would slow down
> packages that do not need them.

As far as I have tested, my suggestion behaves exactly like the
current one. The only difference is that when several variables are
unset, the related dpkg-{architecture,buildflags,..} tool is called
only once.

> Ideally, most of this would not be needed at all, because we could
> rely on dpkg-buildpackage having exported all of these, which it
> already needs to compute in most cases anyway, but this is something
> we cannot have in Debian for now. I'm pondering adding a mode, not to
> be used by Debian, which could be used by local packages or other
> distros though.

A variable like DEB_BUILD_FLAGS_ALREADY_SET could shortcut all tests,
but I wonder if the speed gain would be worth the added complexity,
assuming that each dpkg-{architecture,buildflags,...} is called at
most once and only if at least a variable is both unset and actually
read.

> I'll go over these in the coming days.

I will do my best to rebase before that :-)

[toc] | [prev] | [standalone]

Back to top | Article view | linux.debian.bugs.dist

csiph-web