On 09/10/13 14:46, Allan McRae wrote: > On 09/10/13 14:40, Allan McRae wrote: >> On 22/09/13 20:25, Ashley Whetter wrote: >>> Moved functions out in preparation for splitting out download functions. >>> scripts/libmakepkg/*.sh files only import the files from their relevant >>> directory. >>> All libmakepkg files have an inclusion guard. >>> Also added libmakepkg targets to Makefile.am. >>> >>> Signed-off-by: Ashley Whetter <[email protected]> >>> --- >> >> Patch is fine but copyright years are excessive in some cases. As far >> as I can tell, no line has been left untouch by contributors not covered >> by the "Pacman Development Team" banner (apart form function names, >> brackets and whitespace. So we don not need to propegate those to the >> new files. Changes I will make while pulling this are noted below. >> >>> scripts/.gitignore | 4 + >>> scripts/Makefile.am | 38 +++++++- >>> scripts/libmakepkg/util.sh.in | 30 +++++++ >>> scripts/libmakepkg/util/message.sh | 55 ++++++++++++ >>> scripts/libmakepkg/util/url.sh.in | 145 ++++++++++++++++++++++++++++++ >>> scripts/libmakepkg/util/util.sh.in | 58 ++++++++++++ >>> scripts/makepkg.sh.in | 180 >>> ++----------------------------------- >>> 7 files changed, 333 insertions(+), 177 deletions(-) >>> create mode 100644 scripts/libmakepkg/util.sh.in >>> create mode 100644 scripts/libmakepkg/util/message.sh >>> create mode 100644 scripts/libmakepkg/util/url.sh.in >>> create mode 100644 scripts/libmakepkg/util/util.sh.in >>> > > Final note. New files needed to be added to scripts/po/POTFILES.in. > Done on my working branch. >
Final, final note. There are issues in the Makefile that need fixed before I can pull this. Fix that, and address the comments in my previous replies, and I will merge. > diff --git a/scripts/Makefile.am b/scripts/Makefile.am > index 8130704..4c52bd4 100644 > --- a/scripts/Makefile.am > +++ b/scripts/Makefile.am > @@ -36,13 +36,22 @@ LIBRARY = \ > library/size_to_human.sh \ > library/term_colors.sh > > +LIBMAKEPKG = \ > + $(LIBMAKEPKG_INC) \ > + util/message.sh > + > +LIBMAKEPKG_INC = \ > + util.sh \ > + util/url.sh \ > + util/util.sh > + message.sh does not have the library include field, but still needs modified via the $(edit) below. Join these together. Also, I guess this needs added to EXTRA_DIST to stop "make dist" breaking. Please check. > # Files that should be removed, but which Automake does not know. > MOSTLYCLEANFILES = $(bin_SCRIPTS) > > libmakepkgdir = $(libdir)/makepkg > > clean-local: > - $(AM_V_at)$(RM) -r .lib > + $(AM_V_at)$(RM) -r .lib $(addprefix libmakepkg/,$(LIBMAKEPKG_INC)) > > if USE_GIT_VERSION > GIT_VERSION := $(shell sh -c 'git describe --abbrev=4 --dirty | sed s/^v//') > @@ -83,7 +92,20 @@ $(OURSCRIPTS): Makefile > $(AM_V_at)chmod +x,a-w $@ > @$(BASH_SHELL) -O extglob -n $@ > > +$(addprefix libmakepkg/,$(LIBMAKEPKG_INC)): Makefile > + $(AM_V_at)$(RM) $@ > + $(AM_V_GEN)test -f $(srcdir)/[email protected] && m4 -P -I $(srcdir) $(srcdir)/[email protected] | $(edit) >$@ Currently, we do not need to run these through m4. So: $(AM_V_GEN)test -f $(srcdir)/[email protected] && $(edit) $(srcdir)/[email protected] >$@ > + $(AM_V_at)chmod a-w $@ > + @$(BASH_SHELL) -O extglob -n $@ > + > +libmakepkg: \ > + $(srcdir)/libmakepkg/util.sh \ > + $(srcdir)/libmakepkg/util/message.sh \ > + $(srcdir)/libmakepkg/util/url.sh \ > + $(srcdir)/libmakepkg/util/util.sh > + > makepkg: \ > + libmakepkg \ > $(srcdir)/makepkg.sh.in \ > $(srcdir)/makepkg-wrapper.sh.in \ > $(srcdir)/library/parseopts.sh > @@ -131,7 +153,12 @@ makepkg-wrapper: \ > $(srcdir)/makepkg-wrapper.sh.in \ > $(srcdir)/makepkg.sh.in \ > $(srcdir)/library/parseopts.sh \ > - | makepkg > + $(srcdir)/libmakepkg/util.sh \ > + $(srcdir)/libmakepkg/util/message.sh \ > + $(srcdir)/libmakepkg/util/url.sh \ > + $(srcdir)/libmakepkg/util/util.sh \ > + | libmakepkg \ > + makepkg You have added libmakepkg as a dependency and an order-only dependency here. Only the latter is needed. And there is no need to add the files from libmakepkg, as they are covered already. > $(AM_V_at)$(MKDIR_P) .lib > $(AM_V_at)mv -f makepkg .lib > $(AM_V_at)$(RM) $@ > @@ -146,6 +173,10 @@ install-data-hook: > cd $(DESTDIR)$(bindir) && \ > $(RM) makepkg makepkg-wrapper > $(INSTALL) .lib/makepkg $(DESTDIR)$(bindir)/makepkg > + $(AM_V_at)$(MKDIR_P) $(DESTDIR)$(libmakepkgdir)/util > + for lib in $(LIBMAKEPKG); do \ > + $(INSTALL) libmakepkg/$$lib $(DESTDIR)$(libmakepkgdir)/$$lib; \ > + done > cd $(DESTDIR)$(bindir) && \ > $(RM) repo-elephant && \ > ( $(LN_S) repo-add repo-elephant || \ > @@ -160,5 +191,8 @@ install-data-hook: > uninstall-hook: > cd $(DESTDIR)$(bindir) && \ > $(RM) repo-remove repo-elephant > + for lib in $(LIBMAKEPKG); do \ > + $(RM) -r $(DESTDIR)$(libmakepkgdir)/$$lib; \ What is the -r doing. The directories are not in $(LIBMAKEPKG) so they will not be removed. > + done > > # vim:set ts=2 sw=2 noet:
