Re: bug: git-sh-setup should not be in $PATH

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Han-Wen Nienhuys wrote:
> Jakub Narebski escreveu:
> 
>> Please add some longer commit message.
> 
> do you have any specifics you would like me to mention?

More detailed description what "Allow non-srcdir builds using cd
$builddir && $srcdir/configure" mean and why we might want to do that.

For example:

  We might want to build git from outside source directory. For this we
  need...

By the way, I think this patch is about _two_ changes. Allow to build
outside source directory, by providing srcdir AND separate change to
allow builds of separate parts of git, with separate Makefile, to
include user-generated configuration file config.mak and ./configure
generated config.mak.autogen configuration file.

Shouldn't this patch be split into two?

>>>  exec_prefix = @exec_prefix@
>>>  bindir = @bindir@
>>> -#gitexecdir = @libexecdir@/git-core/
>>> -datarootdir = @datarootdir@
>>> -template_dir = @datadir@/git-core/templates/
>>>  
>>>  mandir=@mandir@
>> 
>> Why have you removed setting datarootdir and template_dir? I would
>> have thought that you would rather change it to
>> 
>>   #gitexecdir = @libexecdir@/git-core/
>>   datarootdir = @datarootdir@
>>   GIT_datadir = @datadir@/git-core/
>>   template_dir= @datadir@/git-core/templates/
> 
> The Makefile already has this code, so adding it here is duplication
> of work and code.

Good call. Well, at least now, when ./configure script doesn't provide
means to change template_dir etc. from command line, via options.

Still I think that at least

  datarootdir = @datarootdir@

should be not removed.

> If you think putting code in the generated file is a good idea, I
> propose we just generate the entire Makefile, as is the standard usage
> for autoconf.

The stance on autoconf is that it has to be _optional_ part of
compilation. And I think it does good job of this.

>>> +## generate subdirectories and sub Makefiles. 
>>> +for d in `cd $srcdir &&  find . -type d -print | grep -v '\.git'` ;
>>> +do
>>> +  if test ! -d  $d ; then
>>> +    echo creating $d 
>>> +    mkdir $d
>>> +  fi
>>> +  
>>> +  if test -f $srcdir/$d/Makefile ; then
>>> +    
>>> +    dnl [[]] is to keep m4 happy
>>> +    depth=`echo $d/ | sed -e 's!^\./!!g' -e 's![[^/]]*/!../!g'`
>>> +    echo creating $d/Makefile
>>> +    cat << EOF> $d/Makefile
>>> +include ${depth}config.mak.autogen
>>> +here-srcdir=\$(srcdir)/$d/
>>> +VPATH=\$(here-srcdir)
>>> +include \$(here-srcdir)/Makefile
>>> +EOF
>>> +
>>> +  fi 
>>> +done
>>> +exit 1
>> 
>> What is this for? The ./configure script, generated by autoconf from
>> configure.ac (by "make configure"), generates config.mak.autogen file
>> from config.mak.in, which is included in main (top) Makefile.
> 
> in some cases, the files can also be called stand alone, eg.
> 
>   [lilydev@haring perl]$ pwd
>   /home/lilydev/vc/git/perl
> 
>   [lilydev@haring perl]$ make
>   make -f perl.mak all
>   make[1]: Entering directory `/home/lilydev/vc/git/perl'
>   make[1]: Leaving directory `/home/lilydev/vc/git/perl'
> 
> The above Makefile generation makes sure that this behavior is
> mirrored in the builddir. Also, I'm not sure if the vpath settings get
> exported automatically.
> 
> Having multiple Makefiles in the builddir is the standard behavior for
> autotool'ed packages. 

Git is not autotool'ed package. Autoconf is used only for _optional_
compile and installation configuration.

>> The variables defined in config.mak.autogen are of course visible in
>> make in subdirectories (make invoked from main makefile). Why the
>> change? What about user-generated config.mak?
> 
> good point. I'll include it too.
> 
>> This part IMHO has no sense, and has no place here.

If you want to create or modify Makefiles, do that. Not generate
"minimal" Makefiles in every and each subdirectory.

You would be able to compile git fully only from top directory. So what?
-- 
Jakub Narebski
Poland
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]