Re: [PATCH] Allow generating a non-default set of documentation
Junio C Hamano gits...@pobox.com writes: Jeff King p...@peff.net writes: On Sun, Oct 07, 2012 at 04:25:58PM -0700, Junio C Hamano wrote: Yeah, modulo that the defaults is tracked and does not have to have the dash before include (it is an error if it is missing, no?). It may want to be named with s/defaults/autodetect/, though. diff --git a/Makefile b/Makefile index e3e3cd5..c00fd32 100644 --- a/Makefile +++ b/Makefile ... -endif - +-include config.mak.defaults -include config.mak.autogen -include config.mak Yeah, sorry, mindless copying on my part from the lines below. It clearly should not have the -. I'm fine with something besides defaults. I meant it to be these are the config defaults before you tweak them, but yeah, it is more like these are the config options we picked up from uname. Let's put this on hold in the possibly a good direction to go in pile, and defer it to post 1.8.0; I haven't even looked at (and do not plan to before the release) these auto-detect bits are all safe to be included in Makefiles in subdirectories. Oh by the way, by this (on hold) I meant not just your move the default-by-platform to a separate include, but the whole a platform may not want manpages topic. The patch posted was merely me wanting to format git-push.txt and nothing else when sanity checking a patch to that file, and it is insufficient for the purpose of a platform may not want manpages without a matching change to the install target. Even though your include thing was primarily done to make it easier to include it from Makefiles in subdirectories, I very much like it even without that benefit. The change makes it clear that there is another platform specific bit after '-include config.mak' (yes, I am looking at you, Darwin), which is a huge eyesore we would want to do something about. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Allow generating a non-default set of documentation
By default, make doc generates the manpages and htmldocs in the Documentation directory, but you may want to change this depending on the target environment, e.g. to include 'pdf'. Introduce a new Makefile variable DEFAULT_DOC_TARGET to allow customizing this. The primary motivation is to let us check documentation patches with $ DEFAULT_DOC_TARGET=git-push.1 make doc but it is not so far-fetched to imagine that Windows users may want to omit manpages with $ DEFAULT_DOC_TARGET=html make doc or somesuch; this lets interested people to enhance the install-doc target in a similar way. Signed-off-by: Junio C Hamano gits...@pobox.com --- Makefile | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git c/Makefile w/Makefile index 8413606..250b87a 100644 --- c/Makefile +++ w/Makefile @@ -299,6 +299,12 @@ all:: # DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR', # DEFAULT_EDITOR='C:\Program Files\Vim\gvim.exe --nofork' # +# You can define DEFAULT_DOC_TARGET to something other than all to change it +# from the built-in default of generating manpages and htmldocs. e.g. +# +# DEFAULT_DOC_TARGET='man html info pdf' +# DEFAULT_DOC_TARGET='html' +# # Define COMPUTE_HEADER_DEPENDENCIES to yes if you want dependencies on # header files to be automatically computed, to avoid rebuilding objects when # an unrelated header file changes. Define it to no to use the hard-coded @@ -1496,6 +1502,8 @@ ifneq (,$(SOCKLEN_T)) BASIC_CFLAGS += -Dsocklen_t=$(SOCKLEN_T) endif +DEFAULT_DOC_TARGET ?= all + ifeq ($(uname_S),Darwin) ifndef NO_FINK ifeq ($(shell test -d /sw/lib echo y),y) @@ -2468,10 +2476,10 @@ $(XDIFF_LIB): $(XDIFF_OBJS) $(VCSSVN_LIB): $(VCSSVN_OBJS) $(QUIET_AR)$(RM) $@ $(AR) rcs $@ $(VCSSVN_OBJS) -export DEFAULT_EDITOR DEFAULT_PAGER +export DEFAULT_EDITOR DEFAULT_PAGER DEFAULT_DOC_TARGET doc: - $(MAKE) -C Documentation all + $(MAKE) -C Documentation $(DEFAULT_DOC_TARGET) man: $(MAKE) -C Documentation man -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Allow generating a non-default set of documentation
On Sun, Oct 07, 2012 at 01:39:32PM -0700, Junio C Hamano wrote: By default, make doc generates the manpages and htmldocs in the Documentation directory, but you may want to change this depending on the target environment, e.g. to include 'pdf'. Introduce a new Makefile variable DEFAULT_DOC_TARGET to allow customizing this. Makes sense (we have DEFAULT_TEST_TARGET for similar reasons). The primary motivation is to let us check documentation patches with $ DEFAULT_DOC_TARGET=git-push.1 make doc Wouldn't it be just as easy to say: $ make -C Documentation git-push.1 ? but it is not so far-fetched to imagine that Windows users may want to omit manpages with $ DEFAULT_DOC_TARGET=html make doc That use case makes a lot more sense to me (or more likely setting it in config.mak). Makefile | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) No change to Documentation/Makefile? So this will work: $ echo DEFAULT_DOC_TARGET=html config.mak $ make doc but this will not: $ cd Documentation $ make Why not do it like this: diff --git a/Documentation/Makefile b/Documentation/Makefile index 267dfe1..ca10313 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -152,7 +152,8 @@ endif endif endif -all: html man +DEFAULT_DOC_TARGET ?= html man +all: $(DEFAULT_DOC_TARGET) html: $(DOC_HTML) which covers both cases? That is also how we handle DEFAULT_TEST_TARGET. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Allow generating a non-default set of documentation
Jeff King p...@peff.net writes: Why not do it like this: diff --git a/Documentation/Makefile b/Documentation/Makefile index 267dfe1..ca10313 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -152,7 +152,8 @@ endif endif endif -all: html man +DEFAULT_DOC_TARGET ?= html man +all: $(DEFAULT_DOC_TARGET) html: $(DOC_HTML) which covers both cases? That is also how we handle DEFAULT_TEST_TARGET. Surely, and thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Allow generating a non-default set of documentation
Jeff King p...@peff.net writes: On Sun, Oct 07, 2012 at 01:39:32PM -0700, Junio C Hamano wrote: ... but it is not so far-fetched to imagine that Windows users may want to omit manpages with $ DEFAULT_DOC_TARGET=html make doc That use case makes a lot more sense to me (or more likely setting it in config.mak). I actually had ifeq ($(uname_S),Windows) at the top-level in mind, not config.mak. I think that is far more important use case than going down to Documentation yourself and run make there (which is not a workflow I deeply care about in the first place). Makefile | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) No change to Documentation/Makefile? So this will work: $ echo DEFAULT_DOC_TARGET=html config.mak $ make doc but this will not: $ cd Documentation $ make Why not do it like this: diff --git a/Documentation/Makefile b/Documentation/Makefile index 267dfe1..ca10313 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -152,7 +152,8 @@ endif endif endif -all: html man +DEFAULT_DOC_TARGET ?= html man +all: $(DEFAULT_DOC_TARGET) html: $(DOC_HTML) which covers both cases? That is also how we handle DEFAULT_TEST_TARGET. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Allow generating a non-default set of documentation
On Sun, Oct 07, 2012 at 03:45:52PM -0700, Junio C Hamano wrote: So here is a proposed update based on your idea, to be squashed on top (i.e. the change to the top-level Makefile in the posted patch is not reverted). Does it make sense? The change to Documentation/Makefile makes sense, but what then is the point of the second half of this hunk from your original: @@ -2468,10 +2476,10 @@ $(XDIFF_LIB): $(XDIFF_OBJS) $(VCSSVN_LIB): $(VCSSVN_OBJS) $(QUIET_AR)$(RM) $@ $(AR) rcs $@ $(VCSSVN_OBJS) -export DEFAULT_EDITOR DEFAULT_PAGER +export DEFAULT_EDITOR DEFAULT_PAGER DEFAULT_DOC_TARGET doc: - $(MAKE) -C Documentation all + $(MAKE) -C Documentation $(DEFAULT_DOC_TARGET) man: $(MAKE) -C Documentation man We know that all is simply a redirect to DEFAULT_DOC_TARGET internally within the documentation Makefile, and we know that it is propagated by the export line above. I do not think it creates the wrong behavior, but it is unnecessary, and omitting a target at all demonstrates to a reader how they can get the same behavior from within Documentation (namely by running just make). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Allow generating a non-default set of documentation
On Sun, Oct 07, 2012 at 03:40:19PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: On Sun, Oct 07, 2012 at 01:39:32PM -0700, Junio C Hamano wrote: ... but it is not so far-fetched to imagine that Windows users may want to omit manpages with $ DEFAULT_DOC_TARGET=html make doc That use case makes a lot more sense to me (or more likely setting it in config.mak). I actually had ifeq ($(uname_S),Windows) at the top-level in mind, not config.mak. I think that is far more important use case than going down to Documentation yourself and run make there (which is not a workflow I deeply care about in the first place). Hmm. Unfortunately that does not work from within Documentation, because Documentation/Makefile never gets to see our default-system tweaks (it sees only config.mak). I know it is a case you do not care about (and nor do I; if I use this at all, it would be to limit my build by setting the variable in my config.mak), but it highlights a subtle issue. The subdir Makefiles receive their config from config.mak.autogen and config.mak, but never get to see any of the default tweaks we do based on $(uname). Which the contents of config.mak could very well depend on, if somebody were trying to be very clever. Would it make sense to pull all of our platform-specific tweaks out into a config.mak.platform (right before config.mak.autogen)? That would be less surprising for cases like this, and I think it would make the Makefile a lot more readable. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Allow generating a non-default set of documentation
On Sun, Oct 07, 2012 at 07:07:03PM -0400, Jeff King wrote: Would it make sense to pull all of our platform-specific tweaks out into a config.mak.platform (right before config.mak.autogen)? That would be less surprising for cases like this, and I think it would make the Makefile a lot more readable. Something like the patch below. Note that you could then base even more decisions on the existing DEFAULT_HELP_FORMAT that is already in the Makefile (and after my patch, in config.mak.defaults). For example, when it is set to html, make both the default build and install targets in Documentation/Makefile do html instead of man. diff --git a/Makefile b/Makefile index e3e3cd5..c00fd32 100644 --- a/Makefile +++ b/Makefile @@ -326,19 +326,6 @@ GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN -include GIT-VERSION-FILE -uname_S := $(shell sh -c 'uname -s 2/dev/null || echo not') -uname_M := $(shell sh -c 'uname -m 2/dev/null || echo not') -uname_O := $(shell sh -c 'uname -o 2/dev/null || echo not') -uname_R := $(shell sh -c 'uname -r 2/dev/null || echo not') -uname_P := $(shell sh -c 'uname -p 2/dev/null || echo not') -uname_V := $(shell sh -c 'uname -v 2/dev/null || echo not') - -ifdef MSVC - # avoid the MingW and Cygwin configuration sections - uname_S := Windows - uname_O := Windows -endif - # CFLAGS and LDFLAGS are for the users to override from the command line. CFLAGS = -g -O2 -Wall @@ -976,513 +963,7 @@ GIT_USER_AGENT = git/$(GIT_VERSION) GIT_USER_AGENT = git/$(GIT_VERSION) -# -# Platform specific tweaks -# - -# We choose to avoid if .. else if .. else .. endif endif -# because maintaining the nesting to match is a pain. If -# we had elif things would have been much nicer... - -ifeq ($(uname_M),x86_64) - XDL_FAST_HASH = YesPlease -endif -ifeq ($(uname_S),OSF1) - # Need this for u_short definitions et al - BASIC_CFLAGS += -D_OSF_SOURCE - SOCKLEN_T = int - NO_STRTOULL = YesPlease - NO_NSEC = YesPlease -endif -ifeq ($(uname_S),Linux) - NO_STRLCPY = YesPlease - NO_MKSTEMPS = YesPlease - HAVE_PATHS_H = YesPlease - LIBC_CONTAINS_LIBINTL = YesPlease - HAVE_DEV_TTY = YesPlease -endif -ifeq ($(uname_S),GNU/kFreeBSD) - NO_STRLCPY = YesPlease - NO_MKSTEMPS = YesPlease - HAVE_PATHS_H = YesPlease - DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease - LIBC_CONTAINS_LIBINTL = YesPlease -endif -ifeq ($(uname_S),UnixWare) - CC = cc - NEEDS_SOCKET = YesPlease - NEEDS_NSL = YesPlease - NEEDS_SSL_WITH_CRYPTO = YesPlease - NEEDS_LIBICONV = YesPlease - SHELL_PATH = /usr/local/bin/bash - NO_IPV6 = YesPlease - NO_HSTRERROR = YesPlease - NO_MKSTEMPS = YesPlease - BASIC_CFLAGS += -Kthread - BASIC_CFLAGS += -I/usr/local/include - BASIC_LDFLAGS += -L/usr/local/lib - INSTALL = ginstall - TAR = gtar - NO_STRCASESTR = YesPlease - NO_MEMMEM = YesPlease -endif -ifeq ($(uname_S),SCO_SV) - ifeq ($(uname_R),3.2) - CFLAGS = -O2 - endif - ifeq ($(uname_R),5) - CC = cc - BASIC_CFLAGS += -Kthread - endif - NEEDS_SOCKET = YesPlease - NEEDS_NSL = YesPlease - NEEDS_SSL_WITH_CRYPTO = YesPlease - NEEDS_LIBICONV = YesPlease - SHELL_PATH = /usr/bin/bash - NO_IPV6 = YesPlease - NO_HSTRERROR = YesPlease - NO_MKSTEMPS = YesPlease - BASIC_CFLAGS += -I/usr/local/include - BASIC_LDFLAGS += -L/usr/local/lib - NO_STRCASESTR = YesPlease - NO_MEMMEM = YesPlease - INSTALL = ginstall - TAR = gtar -endif -ifeq ($(uname_S),Darwin) - NEEDS_CRYPTO_WITH_SSL = YesPlease - NEEDS_SSL_WITH_CRYPTO = YesPlease - NEEDS_LIBICONV = YesPlease - ifeq ($(shell expr $(uname_R) : '[15678]\.'),2) - OLD_ICONV = UnfortunatelyYes - endif - ifeq ($(shell expr $(uname_R) : '[15]\.'),2) - NO_STRLCPY = YesPlease - endif - NO_MEMMEM = YesPlease - USE_ST_TIMESPEC = YesPlease - HAVE_DEV_TTY = YesPlease - COMPAT_OBJS += compat/precompose_utf8.o - BASIC_CFLAGS += -DPRECOMPOSE_UNICODE -endif -ifeq ($(uname_S),SunOS) - NEEDS_SOCKET = YesPlease - NEEDS_NSL = YesPlease - SHELL_PATH = /bin/bash - SANE_TOOL_PATH = /usr/xpg6/bin:/usr/xpg4/bin - NO_STRCASESTR = YesPlease - NO_MEMMEM = YesPlease - NO_MKDTEMP = YesPlease - NO_MKSTEMPS = YesPlease - NO_REGEX = YesPlease - NO_FNMATCH_CASEFOLD = YesPlease - NO_MSGFMT_EXTENDED_OPTIONS = YesPlease - HAVE_DEV_TTY = YesPlease - ifeq ($(uname_R),5.6) - SOCKLEN_T = int - NO_HSTRERROR = YesPlease - NO_IPV6 = YesPlease - NO_SOCKADDR_STORAGE = YesPlease - NO_UNSETENV = YesPlease - NO_SETENV = YesPlease - NO_STRLCPY =
