[darcs-users] darcs patch: Generate SRC_DIRS programmatically. (and 18 more)

Eric Kow kowey at darcs.net
Mon Oct 27 20:32:29 UTC 2008


Hi Trent, David,

Just chipping in to the review for some of the patches in this series.
We might as well simplify the makefile if we're still going to need it
around for some stuff (running tests, generating docs, etc, hoogleindex,
etc)

Generate SRC_DIRS programmatically.
-----------------------------------
ignoring because of the below

Obviate SRC_DIRS altogether.
----------------------------
> - -	rm -f $(foreach dir,$(SRC_DIRS),$(dir)/*.o $(dir)/*.hi)
> +	find -name _darcs -prune -o \( -name \*.o -o -name \*.hi \) -exec rm -f {} +

I am reading this as "ignoring whatever is in _darcs" delete all .o and
.hi files

DARCS_FILES_DEPS is never bound, so don't evaluate it.
------------------------------------------------------
> - -DARCS_FILES := $(DARCS_FILES_DEPS)\
> +DARCS_FILES := \
>  	$(patsubst %,src/%,$(MODULES_AUTOCONF))\
>  	$(patsubst %,src/%,$(MODULES_GENERAL))\
>  	$(patsubst %,src/Crypt/%,$(MODULES_CRYPT))\


Tweak C_OBJS declaration.
-------------------------
> - -C_OBJS := $(patsubst %,src/%,c_compat.o maybe_relink.o atomic_create.o fpstring.o umask.o) \
> - -	src/Crypt/sha2.o src/hscurl.o src/hslibwww.o
> +C_OBJS := $(patsubst %,src/%.o,c_compat maybe_relink atomic_create fpstring umask Crypt/sha2 hscurl hslibwww)

Saves writing '.o' all over the place

Replace procedural := with declarative =.
-----------------------------------------
Makes sense

Leverage gmake's order-only dependencies.
-----------------------------------------
> +# The "| %.dvi" declares an order-only dependency, thereby ensuring
> +# that make -j2 won't run both pdflatex and latex at the same time,
> +# but still allowing PDFs to be built without ever building the PS or
> +# DVI versions.
> +%.pdf : %.tex | %.dvi

Nice... maybe this was what was breaking the parallel make dist?
I thought this might be related to http://bugs.darcs.net/issue506
but I see it has already been closed, nevermind

Remove unpleasant sequencing operators (;) from haddock targets.
----------------------------------------------------------------
Not reviewing, but it does seem cleaner

Add conventional "pdf", "ps" and "html" targets.
------------------------------------------------
This is a refactor of our makefile targets.  Thanks!

Make it obvious why deps are being filtered.
--------------------------------------------
Snipping to the crucial parts...

> - -	haddock ... -h $(filter-out %api-doc-dir,$^)
> +	haddock ... -h $(filter %.hs %.lhs,$^)

And a similar change below.

Fix some predicates I accidentally reversed.
--------------------------------------------
> - -	@test $(HADDOCK_VERSION) -lt 2 || { echo "You need haddock 2.0.0 or later to build the API documentation"; false; }
> +	@test $(HADDOCK_VERSION) -ge 2 || { echo "You need haddock 2.0.0 or later to build the API documentation"; false; }

And that's what I get for now reviewing Trent's (;) patch above :-)

Reduce loquacity of haddock targets.
------------------------------------
>  I think that if someone runs "make api-doc", it's not useful to
>  immediately print

No objections, I think.

Refactor test rules.
--------------------
>  Now the target names correspond to the darcs switches, e.g. "make
>  test-darcs-2" instead of "make test-format2".  There are some legacy
>  pointers so the old targets still work, but they probably put the
>  results in a different directory.

> - -		test-franchise test-old test-hashed test-format2 \
> +		test-franchise \

Don't the new targets need to be phony too?

> +.PHONY: $(addprefix test-,$(formats))
> +.PHONY: $(addprefix bugs-,$(formats))

Oh, nevermind


Use new "ps", "pdf" and "html" targets.
---------------------------------------
> > hunk ./configure.ac 337

Usess the new doc targets in autocnf

Add conventional install-ps/pdf/html targets.
> +installdocs:	install-pdf install-html install-examples
> +$(DESTDIR)$(docdir)/manual $(DESTDIR)$(docdir)/examples:
> +	$(INSTALL) -d $@
> +install-ps:	$(DESTDIR)$(docdir)/manual ps
> +	$(INSTALL_DATA) doc/manual/*.ps $<
> +install-pdf:	$(DESTDIR)$(docdir)/manual pdf
> +	$(INSTALL_DATA) doc/manual/*.ps $<
> +install-html:	$(DESTDIR)$(docdir)/manual html
> +	$(INSTALL_DATA) doc/manual/*.png $<
> +	$(INSTALL_DATA) doc/manual/*.html $<
> +install-examples:	$(DESTDIR)$(docdir)/examples
> +	$(INSTALL_DATA) tools/zsh_completion_new $<
> +	$(INSTALL_DATA) tools/zsh_completion_old $<
> +.PHONY: install-ps install-pdf install-html install-examples

Seems ok on a cursory glance.

Use ANNOUNCE_GHC convention for darcs.
--------------------------------------
Tidier

Generate TEXSOURCES programmatically.
-------------------------------------
> - -TEXSOURCES = preproc src/darcs.lhs src/features.tex \
> - -	src/switching.tex src/configuring_darcs.tex src/gpl.tex \
> - -	$(DARCS_FILES) src/building_darcs.tex \
> - -	src/best_practices.tex src/formats.tex
> +TEXSOURCES = $(shell find src -name \*.tex)
> +TEXSOURCES += preproc $(DARCS_FILES) src/darcs.lhs

Refactor install rules.
-----------------------
>  Importantly, this means that if you just do "make" it will either
>  build PDF or PS, but not both (with a preference for PDF).

>  The "installbin" target has been renamed to "install", since 1) that's
>  the convention, and 2) it was already installing non-binary stuff,
>  namely the bash completion and manpage.
>  
>  Leverages concatenative rules (::) to reduce repetition.

I'm not reviewing this, but I think it can just go in.

> +install::	$(DESTDIR)$(bindir) darcs
> +	$(INSTALL) darcs $<

And if $(DESTDIR)$(bindir) does not exist, I guess it makes it?

Anyway, the (::) thing is a nice trick, thanks.

Only .lhs (not .hs) files could possibly be TeX sources.
--------------------------------------------------------
> - -TEXSOURCES += preproc $(DARCS_FILES) src/darcs.lhs
> +TEXSOURCES += preproc $(filter %.lhs,$(DARCS_FILES)) src/darcs.lhs

Ok

Split darcs.lhs into darcs.tex and darcs.hs.
--------------------------------------------
Does as advertised. I think we can all agree to do this, whether or not
we want to retain the use of literate programming for our user manual.

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.osuosl.org/pipermail/darcs-users/attachments/20081027/411f933e/attachment.pgp 


More information about the darcs-users mailing list