[Build-common-hackers] pd-pkg-tools

[IOhannes zmölnig]

On 12/23/2010 02:42 AM, Jonas Smedegaard wrote:
> Hi IOhannes,
> 
> (sorry for the lat response :-(  )


np.
this time, i'm taking long to respond...

> 
> I would be happy to adopt your code and include it as integral part of
> CDBS.
> 
> Even better, however, would be if you could be persuaded to join and
> help maintain it yourself.  Our list is (apart from the spam,
> apparently) pretty quiet.

fine with me.
i'll subscribe to the list (doing so right now)

tell me what other steps are required on my side.

> 
> 
>> jonas, you have comments on my coding style :-)
> 
> 
> Licensing header refer to FSF postal address.  Better to refer to their
> website - that's how they do themselves nowadays.

ah good to know.
done

> 
> 
> don't check for debhelper.mk - that cause the snippet to require
> specific load order, which is bad.  Instead make targets depending on
> debhelper.mk targets, which then get picked up if debhelper is included.

well, the debhelper checks were there for future use only anyhow. (or
rather it turned out that i didn't need dh anyhow, so i left the few
lines for documentation purposes)


> The comment about code not being specific to other snippets is wrong:
> binary-strip-IMPL/% targets are picked up via other debhelper targets.

i think the comment in question is
 "## code not specific to other backends"
what i meant is, that the code does not depend on a certain snippet
used: e.g. "binary-strip-IMPL/%" can be picked up by debhelper or cmake
or whatever, and it should work regardless (unlike the "ifdef debhelper"
code, which can only be picked up if debhelper is used)

anyhow, since the debhelper stub has been removed, there is no more need
to explicitely note that this part of the code is not specific to
anything so i removed it.

> 
> 
> The variable $(nostrip_package) has a high risk of clashing with user
> environment: Pick a CDBS-specific name instead - e.g.
> $(cdbs_pd_nostrip_package) - or if sensible to reuse across snippets
> then perhaps $(cdbs_nostrip_package) instead.

i changed the name to $(cdbs_pd_common_nostrip_package)
i don't know exactly about reusability (the check as it is right now
tests for both the "nostrip" flag or whether we are building
dbg-libraries (unlikely with pd-externals, acutally)


so i think it might make sense to move the snippet into buildvars.mk,
but i am not so literate with cdbs (and its policies) yet, so i leave
that decision to you.

> There is a test for $(is_debug_package) in an ifeq construct.  That
> won't ever succeed, because ifeq is resolved before that variable is
> defined, and (because it is defined with = not := ) the variable is
> resolved only when used, i.e. inside build targets.

which reminds me: shouldn't the "is_debug_package" variable be called
something more CDBS-specific, like "cdbs_is_debug_package"?

anyhow, since the "is_debug" is currently unlikely to be used by any
pd-specific package i simply removed the offending line.

> 
> Generally ifeq constructs should be avoided as much as possible, as
> should := definitions, as both tend to cause order of snippet inclusions
> to matter, which is bad: Confusing for the end-user at best, and causing
> impossible circular ordering dependencies at worst.

ok.
while i have been working with make for ages, it still has some
mysteries for me :-)

> 
> 
> CDBS_BUILD_DEPENDS_class_pd and CDBS_BUILD_DEPENDS_class_puredata is
> resolved dynamically.  I dislike this, as the intend for these is to
> allow the end-user to suppress specific build-dependencies, and if
> dynamically respolved they are not really specific any longer.

i modelled these after what i found in the other cdbs snippets.
probably i did not grasp their intent (but then i have no idea how i
could have known - at least the documentation doesn't say anything about
these variables related to "suppressing specific build-dependencies")

i'm quite happy to follow any style- and guide-lines, as long as they
are made explicit somewhere.


> 
> If it is related to (unfinished) auto-resolving package-specific
> dependencies, then I recommend putting it in a separate variable from
> the snippet-specific dependencies.
> 
> ...or perhaps I just can't imagine the sane use of that approach.

the original idea was to create a seaparate @cdbs_pd@ variable-expansion
in control.in, but eventually i found how easy it was to add to @cdbs@
so i used that.

> But all this is relatively minor things.  Let's include it, so you can
> start use it - and improve on it as we hack along.

sounds great.
i pushed all my changes to github,
please pick it up from there.

it is still in the shape of a separate debian-pkg, sho it needs some
minor modifications.

>
> So the question remains: Should I take over from here, or will you do it?
> 

if you want me in, i'd prefer to do it together.
i'm not very fluent in cdbs yet, nor does my make knowledge match that
of C/C++.

fmgasdr
IOhannes

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
URL: <http://lists.alioth.debian.org/pipermail/build-common-hackers/attachments/20110104/b2d66448/attachment.pgp>