On Fri, Jun 7, 2013 at 5:28 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Felipe Contreras <felipe.contre...@gmail.com> writes:
>
>> On Fri, Jun 7, 2013 at 5:03 PM, Felipe Contreras
>> <felipe.contre...@gmail.com> wrote:
>>
>>> -all:: $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) 
>>> GIT-BUILD-OPTIONS
>>> +all:: $(ALL_PROGRAMS) $(SCRIPTS_GEN) $(SCRIPT_LIB) $(BUILT_INS) 
>>> $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS
>>
>> Alternatively, we could add $(NO_INSTALL) here.
>
> As ALL_PROGRAMS overlap with most of SCRIPTS_GEN, the above looks
> overly heavy-fisted.  I tend to agree that a separate
>
>         all:: $(NO_INSTALL)
>
> would be much better, assuming that NO_INSTALL will mean "We always
> want to build these, but we never do not want to install them"
> forever (which I am OK to assume).
>
> Also
>
>         make clean
>         make --test=5800 test
>
> did not fail for me, and it turns out that "clean" somehow fails to
> clean git-remote-testpy script.
>
> As git-remote-testpy is only for testing, another possibility is to
> do
>
>     -all:: $(TEST_PROGRAMS) $(test_bindir_programs)
>     +all:: $(TEST_PROGRAMS) $(test_bindir_programs) git-remote-testpy
>
> but I think $(NO_INSTALL) is the cleanest.
>
> Perhaps like this?
>
>  Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index a748133..03fda50 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2239,6 +2239,7 @@ endif
>
>  test_bindir_programs := $(patsubst 
> %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) 
> $(TEST_PROGRAMS_NEED_X))
>
> +all:: $(NO_INSTALL)
>  all:: $(TEST_PROGRAMS) $(test_bindir_programs)
>
>  bin-wrappers/%: wrap-for-bin.sh
> @@ -2489,7 +2490,7 @@ clean: profile-clean coverage-clean
>         $(RM) *.o *.res block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o 
> xdiff/*.o vcs-svn/*.o \
>                 builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB)
>         $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X
> -       $(RM) $(TEST_PROGRAMS)
> +       $(RM) $(TEST_PROGRAMS) $(NO_INSTALL)
>         $(RM) -r bin-wrappers $(dep_dirs)
>         $(RM) -r po/build/
>         $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h 
> $(ETAGS_TARGET) tags cscope*

Looks good to me.

-- 
Felipe Contreras
--
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

Reply via email to