On Sun, Dec 05, 2010 at 05:32:43PM -0400, Joey Hess wrote:
> Roger Leigh wrote:
> > Another possibility to consider.  Rather than overriding the various
> > dh_ commands with override, which is useful to invoke them with
> > extra/different options, it would be possible to additionally add
> > _hook or _local rules, possibly with _indep and _arch variants e.g.
> > dh_install_arch_hook to be run after dh_install.  This prevents the
> > need for an override_dh_install.  For example:
> 
> Sorry to say I see no need for this. It adds three more things have to
> be learned to use dh. I purposefully want to keep dh's surface area as
> small as possible. 
> 
> The current override targets           With your hooks, there is no
> are nice because you can start         common case to start off with.
> off with an override that does         What hook will be needed depends
> not change behavior, and which         on exactly what needs to be changed.
> an editor can easily be taught
> to insert:
> 
> dh_override_dh_foo:
>       dh_foo

Agreed.  The key point of the "hook" concept is that it's not in any
way overriding the behaviour of the various dh* commands in the
manner of override.  It's /supplementing/ them.  And it's entirely
complementary to override (you can use both independently).  I
agree entirely there is overlap between them.

[…]
> Maybe instead of the option you        With your hooks, I have to remember
> need to run something before;          that _hook runs _after_ (and there's
> it's obvious to anyone who knows       nothing that runs before, so still
> make how to change it to do that:      cannot use them:
> 
> dh_override_dh_foo:                    dh_override_dh_foo:
>         something                              something
>         dh_foo                                 dh_foo

This is something I considered, but chose not to implement in this
initial proof-of-concept patch.  The reasons were:

- running something before is not as common as running after
- this can be implemented using make dependencies to trigger
  actions beforehand
- it's trivial to add should it be required

> And if you decide it needs to run      Now I can use your hook, but I have
> after instead, that's a single yank    to change everything I had written:
> and paste to do:
> 
> dh_override_dh_foo:                    dh_foo_hook:
>         dh_foo                                something
>         something
>                                        (And, looking at that, it's not really
>                                        clear in which order it and dh_foo 
> run.)
> 
> Oh, I still needed that -X after       Now I have to write another target,
> all, of course it's trivial to put     and I also have to consider whether
> back:                                  I can mix the _hook with the
>                                        override target.
> 
> dh_override_dh_foo:                    dh_override_dh_foo:
>         dh_foo -X                              dh_foo -X
>         something                      dh_foo_hook:
>                                                something

I agree that these examples show maintenance as being more complex.
However, they aren't really using the hooks/overrides in the manner
I intended.  My intentions were to make use of dh much simpler, by
cleanly separating overriding of the dh commands and any additional
tasks.  You examples above are using the override rule to do both
these things, which is contrary to how I envisioned their use (though
it is obviously perfectly acceptable to continue to use them like
this).

If we had separate override, "pre-hook" and "post-hook" targets
(I'm sure better names are possible), then you can keep everything
separate and avoid the complications illustrated above.

> > override_dh_auto_install:
> > ifneq (,$(shell dh_listpackages -a 2>/dev/null))
> >     $(MAKE) -C debian/build install DESTDIR=$(CURDIR)/debian/install
> > endif
> > ifneq (,$(filter schroot-common, $(shell dh_listpackages)))
> >     $(MAKE) -C debian/build/po install DESTDIR=$(CURDIR)/debian/install
> > endif
> > 
> > would become
> > 
> > dh_auto_install_arch_hook:
> >     $(MAKE) -C debian/build install DESTDIR=$(CURDIR)/debian/install
> > 
> > dh_auto_install_indep_hook:
> >     $(MAKE) -C debian/build/po install DESTDIR=$(CURDIR)/debian/install
> 
> At the moment we don't know for how many packages it will make sense
> to do this splitting. It might be 0.1% for all I know. I'd rather wait
> until there is a demonstrated wide need for a simpler syntax. Then there
> would be other ways to simplify it, for example:
> 
>       override_dh_auto_install:
>               dh_do -a -- $(MAKE) -C debian/build install 
> DESTDIR=$(CURDIR)/debian/install
>               dh_do -i -- $(MAKE) -C debian/build/po install 
> DESTDIR=$(CURDIR)/debian/install

The schroot rules file I included as an example was made much simpler
with the addition of the above hooks, though the correct invocation of
the build and install targets (in my other patch) from the dh binary
sequence does mean a lot of the complexity can go directly into the
standard targets rather than into overrides.  I'll probably adopt dh
in schroot once the target invocation patch is applied; this one is
not as essential, just an extra "nice to have" addition.

The principal motivation for this change was to eliminate the use of

  ifneq (,$(shell ...))

logic from the rules file, which isn't very readable.  Another
alternative might be a dh_is_building script (or similar) which could
be used like this:

target:
        if dh_is_building foo; then \
          do something-for-foo; \
        fi

or

target:
        if dh_is_building -a; then \
          do arch-all-stuff; \
        fi

This moves the conditional from make to the shell.


Regards,
Roger

-- 
  .''`.  Roger Leigh
 : :' :  Debian GNU/Linux             http://people.debian.org/~rleigh/
 `. `'   Printing on GNU/Linux?       http://gutenprint.sourceforge.net/
   `-    GPG Public Key: 0x25BFB848   Please GPG sign your mail.

Attachment: signature.asc
Description: Digital signature

Reply via email to