v2, 5 partscontrib/subtree/Makefile: Standardisation pass

contrib/subtree/Makefile is a shambles in regards to it's consistency with other makefiles, which makes subtree overly painful to include in build scripts.

The main issues are that calls are made to git itself in the build process, and that a subtree-exclusive variable is used for specifying the exec path. Patches 1/5 through 3/5 resolve these.

The "cleanup" fixes (4/5 and 5/5) are based on precedents set by other makefiles across the project.

One problem is foreseen: 3/5 will necessitate that package maintainers who already have git-subtree included in their packages update their build-scripts.

Signed-off-by: James Denholm <nod.helm@gmail.com>
Based-on-patch-by: Dan McGee <dpmcgee@gmail.com>

James Denholm (5):
  contrib/subtree/Makefile: scrap unused $(gitdir)
  contrib/subtree/Makefile: Use GIT-VERSION-FILE
  contrib/subtree/Makefile: s/libexecdir/gitexecdir
  contrib/subtree/Makefile: Doc-gen rules cleanup
  contrib/subtree/Makefile: clean rule cleanup

 contrib/subtree/Makefile | 40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

All references were removed in 7ff8463dba0d74fc07a766bed457ae7afcc902b5, but the assignment itself wasn't. Hence, drop gitdir assignment.

Signed-off-by: James Denholm <nod.helm@gmail.com>
---
 contrib/subtree/Makefile | 1 -
 1 file changed, 1 deletion(-)

GVF is already being used in most/all other makefiles in the project, and has been for _quite_ a while. Hence, drop file-unique gitver and replace with GIT_VERSION.

Signed-off-by: James Denholm <nod.helm@gmail.com>
---
 contrib/subtree/Makefile | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

$(libexecdir) isn't used anywhere else in the project, while $(gitexecdir) is the standard in the other appropriate makefiles. Hence, replace the former with the latter.

Signed-off-by: James Denholm <nod.helm@gmail.com>
---
 contrib/subtree/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

git:Documentation/Makefile establishes asciidoc/xmlto calls as being handled through their appropriate variables, Hence, change to bring into congruency with.

Similarly, MANPAGE_XSL exists in git:Documentation/Makefile, while MANPAGE_NORMAL_XSL does not outside contrib/subtree. Hence, replace MANPAGE_NORMAL_XSL with MANPAGE_XSL.

Signed-off-by: James Denholm <nod.helm@gmail.com>
---
 contrib/subtree/Makefile | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

git:Documentation/Makefile and others establish "RM ?= rm -f" as a convention for rm calls in clean rules, hence follow this convention instead of simply forcing clean to use rm.

subproj and mainline no longer need to be removed in clean, as they are no longer created in git:contrib/subtree by "make test". Hence, remove the rm call for those folders.

Other makefiles don't remove "*~" files, remove the rm call to prevent unexpected behaviour in the future. Similarly, clean doesn't remove the installable file, so rectify this.

Signed-off-by: James Denholm <nod.helm@gmail.com>
---

Admittedly, git:Makefile does not itself follow the "RM ?= rm -f" setup, instead using "RM = rm -f", but I felt that there were probably $ARCANE_REASONS for this.

Also, Peff, you were right about the dirs.

 contrib/subtree/Makefile | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

On Sat, May 03, 2014 at 10:49:35PM +1000, James Denholm wrote:

I do not think BSD-ism matters for "rm", as it works pretty much the same everywhere. "install", on the other hand, is a bit weirder between systems. So you might want to leave that comment as-is.

OTOH, we do not even bother with such a comment in the main Makefile.

-Peff

On 5 May 2014 15:09:39 GMT+10:00, Jeff King <peff@peff.net> wrote:

True. I might just buff that out when sending the patch to Junio, unless protocol dictates otherwise - a reroll for a single comment line seems a bit excessive to me at the moment.

Regards, James Denholm.

On Tue, May 06, 2014 at 07:41:29AM +1000, James Denholm wrote:

I don't think it is that big a deal either way.

It's fine to tweak when you send re-roll the final for Junio. Sometimes for trivial fixups like this, Junio can just tweak it as he applies, but I do not know if he is even paying attention to this thread, so you may want to re-post anyway to get his attention.

Either way, feel free to add my:

  Reviewed-by: Jeff King <peff@peff.net>

-Peff

On 6 May 2014 07:49:30 GMT+10:00, Jeff King <peff@peff.net> wrote:

Sure, sounds good and will do.

Awesome, thanks again.

Regards, James Denholm.

On Sat, May 03, 2014 at 10:49:30PM +1000, James Denholm wrote:

Thanks, these all look sane to me (I do not use subtree, but since it's just about Makefiles, it was pretty easy to review).

I think that's probably OK. We strive for backwards compatibility in the tool itself, but refactoring Makefiles in contrib/ affects a pretty limited audience.

-Peff

On 5 May 2014 15:08:04 GMT+10:00, Jeff King <peff@peff.net> wrote:

Thanks for the review!

Given that subtree subtree doesn't really generate a lot of discussion, would it be advisable to wrap this up (barring further discussion) and send it off to Junio rather than waiting for further community consensus?

Regards, James Denholm.

[fixed David's address in cc list]

On Tue, May 06, 2014 at 07:54:30AM +1000, James Denholm wrote:

I do not know if "lack of discussion" is a good reason to consider something in good shape; oftentimes it is a sign that nobody is interested in the area. We usually rely on "area maintainers" to give an OK to the patches if they are not something that the maintainer himself has an interest in.

However, in this case, you did get review, and I think it is pretty easy to see the patches are good even if one does not care about the particular area. So I think they are fine to pass on and apply.

Note also that patches like this are a great place to get started, as they help build trust in a contributor, who can later help out with area maintenance.

-Peff

On 6 May 2014 08:01, Jeff King <peff@peff.net> wrote:

Ah, right. Wasn't sure what was going on there.

Yeah, I certainly only meant that in the context of this particular patch, post-review.

Sounds good, I'll send it on up now. Thanks again for the help.

Yeah, to be honest, beyond the immediate goal of getting subtree in more distros, that is kinda the plan. A bit of a practical experience in contributing to the project, learning the specific ropes and such before proposing more substantial discussion and fixes/changes.

--- Regards, James Denholm.