<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 TRANSITIONAL//EN">
<HTML>
<HEAD>
<META HTTP-EQUIV="Content-Type" CONTENT="text/html; CHARSET=UTF-8">
<META NAME="GENERATOR" CONTENT="GtkHTML/3.30.3">
</HEAD>
<BODY>
Hello<BR>
<BR>
Le samedi 25 décembre 2010 à 22:18 +0100, Jonas Smedegaard a écrit :
<BLOCKQUOTE TYPE=CITE>
<PRE>
There are a few trailing spaces that should be removed.
</PRE>
</BLOCKQUOTE>
Done<BR>
<BR>
<BLOCKQUOTE TYPE=CITE>
<PRE>
I suggest to merge [waf-vars.mk] with the main waf.mk file
</PRE>
</BLOCKQUOTE>
Done
<PRE>
</PRE>
<BLOCKQUOTE TYPE=CITE>
<PRE>
I recommend to support global, package-default and per-package vars
</PRE>
</BLOCKQUOTE>
I use $(cdbs_curdestdir) in common-install-impl target. I tried something with calling cdbs_expand_curvar, but no success.<BR>
the var isn't expanded as I expected. I commented out for the moment<BR>
<BR>
<BLOCKQUOTE TYPE=CITE>
<PRE>
Instead of explicitly touching stampfiles, it is more elegant to do
"touch $@" (expands to current build target so essentially do the same).
</PRE>
</BLOCKQUOTE>
<PRE>
Done
</PRE>
<BR>
<BLOCKQUOTE TYPE=CITE>
<PRE>
You mention that to use it needs including debhelper.mk. That's not
true - it works perfectly fine without CDBS'ifying debhelper too.
</PRE>
</BLOCKQUOTE>
building and staging works, but cleaning let some files in the debian dir (the stage is one of them)<BR>
and the binary deb file isn't built without debhelper.mk. Is this behaviour desired ?<BR>
<BR>
<BR>
<BLOCKQUOTE TYPE=CITE>
<PRE>
Please keep comment lines below 72 chars. Code lines are ok to be
longer if necessary.
</PRE>
</BLOCKQUOTE>
<PRE>
Done
</PRE>
<BR>
<BLOCKQUOTE TYPE=CITE>
<PRE>
I dislike your "define" contruct. It is resolved early like ifeq
constructs, which is generally problematic as is force a specific order
of snippet inclusions etc. This particular one does no harm but I would
prefer to avoid it anyway for the sake of discouraging those - and it
seems to me that it could just as well be done as a simple variable:
checkwaf = test -f ./waf -a -x ./waf
Oh, and there's another problem in that you use a short variable name
with a high risk of clashing with user code. Better to always include a
leading "cdbs_" and also include a snippet-specific keyword to avoid
clashing with other parts of CDBS. Like this:
cdbs_waf_checkwaf = test -f ./waf -a -x ./waf
...but really it is more elegant IMO a) do the checks separately (the -a
might not be POSIX compliant although in recent Debian this restraint
might be relaxed) and b) only check in initial targets. Like this:
# ensure waf is executable
pre-build clean::
test -f ./waf
test -x ./waf
</PRE>
</BLOCKQUOTE>
Done. I created a new target (cdbs-waf-sanity-check) upon which depends configure and clean targets<BR>
<BR>
<BLOCKQUOTE TYPE=CITE>
<PRE>
As a sidenote, I really dislike the fundamental design of waf: Shipping
a self-extracting scripting environment, obfuscated by an odd format,
and telling users to execute that code blindly. It is a security risk!
We really should extend the initial check to e.g. keep a checksum
</PRE>
</BLOCKQUOTE>
Do you mean keeping track of a checksum in cdbs source code ?<BR>
If yes, it means we must keep an eye on every waf new release. I don't think it could work on long term.<BR>
<BR>
<BLOCKQUOTE TYPE=CITE>
<PRE>
Much better would be a separately packaged waf tool. This has been
tried, but upstream obstructed that to the extend that it has now been
dropped from Debian.
</PRE>
</BLOCKQUOTE>
This would be ideal, but the Waf user guide itself discourage a system-wide installation and advice to ship it in the tarball source tree<BR>
<BR>
<BR>
<BLOCKQUOTE TYPE=CITE>
<PRE>
Instead of only mentioning in documentation that waf needs Python, it is
more elegant to support build-dependency auto-resolving. Look at
CDBS_BUILD_DEPENDS* vars in many of the other snippets, and (as with
everything else) just ask if it isn't obvious how to do it.
</PRE>
</BLOCKQUOTE>
<PRE>
Done
</PRE>
<BR>
<BR>
<BLOCKQUOTE TYPE=CITE>
<PRE>
Please add yourself as "Uploader" in debian/control.in.
</PRE>
</BLOCKQUOTE>
Done<BR>
<BR>
<BR>
<BLOCKQUOTE TYPE=CITE>
<PRE>
That's it. Can't find any more to complain about just now :-)
- Jonas
_______________________________________________
Build-common-hackers mailing list
<A HREF="mailto:Build-common-hackers@lists.alioth.debian.org">Build-common-hackers@lists.alioth.debian.org</A>
<A HREF="http://lists.alioth.debian.org/mailman/listinfo/build-common-hackers">http://lists.alioth.debian.org/mailman/listinfo/build-common-hackers</A>
</PRE>
</BLOCKQUOTE>
<BR>
Rémi
</BODY>
</HTML>