Re: [PATCH GSoC] gitweb: Add global installation target for gitweb

[Pavan Kumar Sunkara <pavan.sss1991@gmail.com>]

>
> As I said, I agree with the reasoning and the fact that patch is split
> into two. What I disagree with is the splitting itself.
>
> Making 'install-gitweb' prerequisite to 'install' target, or in other
> words adding installing gitweb *somewhere*, should in my opinion by in
> this second patch.
>
>> I choose Makefile rather than choosing gitweb/Makefile becuase
>> git-instaweb is a package of git not a package of gitweb. So, I choose
>> Makefile rather than choosing gitweb/Makefile.
>
> This is important fact; the description that 'gitwebdir' has default
> value set (also) in Makefile because of the future change to the way
> git-instaweb is build / installed should be in commit message of a new
> first patch.
>
>
> BTW. if 'gitwebdir' would be set to some default value both in Makefile
> and in gitweb/Makefile, then in gitweb/Makefile the "?=" operator must
> be used (set if undefined).
>
> Note that default value in Makefile can be "$(sharedir)/gitweb", and in
> gitweb/Makefile can be '/var/www/cgi-bin'.
>
> [...]
>> > Two things.
>> >
>> > First, I think providing default value for 'gitwebdir' could be I good
>> > idea.  I think that all other build variables containing installation
>> > directories have default values.  But I do wonder whether the
>> > "$(sharedir)/gitweb" is a good default value for 'gitwebdir' (see also
>> > comment about git-instaweb below).
>>
>> I chose it because the lib files of gitk and git-gui are being
>> installed in sharedir.
>
> O.K., I can agree with this. You might want to put this substantiation
> in the commit message, though.
>
>> > Second, the issue with git-instaweb in its current form, and splitting
>> > gitweb is very important.  I am not sure though if this is correct
>> > solution for this problem.  It is NOT A FULL SOLUTION, that is sure.
>>
>> Yes, It is not a full solution. There is another patch that is
>> currently in discussion with my mentors.
>>
>> Petr and Christian told me to make sure that I send patches as small
>> as possibl so that they will be merged into the mainstream. That is my
>> GSoC aim too.
>>
>> So, I sent this small patch which just installs gitweb with git's
>> "make install" without breaking the git-instaweb.sh script.
>> The patch for modifying git-instaweb will be sent soon to his mailing list.
>
> That is a good practice, and a good idea.
>
> My complaints are about putting too much in this first patch (I think
> that making 'install' target install gitweb should be put in the commit
> that changes git-instaweb, as those two changes have to be
> synchronized), and about that commit message seems to claim that this
> commit does more than it really does.
>
> --
> Jakub Narebski
> Poland
>

So, what I need to do in this patch is add default values. Not
installing in 'install' target. ??
Or any thing else.

If you could specify what needs to be done, I will be happy resend the patch.

- Pavan