Re: [PATCH v6 2/5] git-remote-mediawiki: new git bin-wrapper for developement

benoit.person@ensimag.fr writes:

I really do not like the smell of this.

As far as I can tell, the only real reason why you need this and
cannot use ../../bin-wrappers/git directly is because the GITPERLLIB
it gives you only points at ../../perl/blib/lib and not this
directory.

Two possible alternatives:

 - Is there a reason you would not want to "install" whatever Perl
   modules you want to "use" via GITPERLLIB mechanism to
   ../../perl/blib/lib?  Perhaps it will interfere with the real
   installation step in ../../perl/Makefile?  If that is the case,
   then it is not a good idea, but otherwise, that would let you use
   ../../bin-wrappers/git as-is.

 - Perhaps we could do:

	GITPERLLIB="${GPLEXTRA+$GPLEXTRA:}@@BUILD_DIR@@/perl/blib/lib"

   in wrap-for-bin.sh, so that your instruction can become

	GPLEXTRA=$(pwd) ../../bin-wrappers/git whatever-mw-thing

   and you do not have to have this file?  We would also need to
   "unset GPLEXTRA" at the beginning of test-lib.sh if we were to do
   this.

How does a developer (or user) use this script?  From your Makefile
(e.g. "make test")?  Manually following some written instruction?
In either case, the latter option feels a lot more sensible
alternative without having to maintain this extra copy of wrap-for-bin
that can easily go out of sync.

+
+
+GIT_ROOT_DIR=../../..
+GIT_EXEC_PATH=$(cd "$(dirname "$0")" && cd ${GIT_ROOT_DIR} && pwd)
+
+echo $GIT_EXEC_PATH
+
+if test -n "$NO_SET_GIT_TEMPLATE_DIR"
+then
+	unset GIT_TEMPLATE_DIR
+else
+	GIT_TEMPLATE_DIR="$GIT_EXEC_PATH"'/templates/blt'
+	export GIT_TEMPLATE_DIR
+fi
+# Hack to make the `use lib` call works with multiple paths
+GITPERLLIB="$GIT_EXEC_PATH"'/contrib/mw-to-git:'"$GIT_EXEC_PATH"'/perl/blib/lib'
+GIT_TEXTDOMAINDIR="$GIT_EXEC_PATH"'/po/build/locale'
+PATH="$GIT_EXEC_PATH"'/contrib/mw-to-git:'"$GIT_EXEC_PATH"'/bin-wrappers:'"$PATH"
+export GIT_EXEC_PATH GITPERLLIB PATH GIT_TEXTDOMAINDIR
+
+exec "${GIT_EXEC_PATH}/git" "$@"

As far as I can tell, the only real reason why you need this and
cannot use ../../bin-wrappers/git directly is because the GITPERLLIB
it gives you only points at ../../perl/blib/lib and not this
directory.

Plus (forgot to mention it in the other mail :/ ) it enables us to not
"copy" git-mw and git-remote-mediawiki at each "make" and stop us from
polluting the toplevel directory with those two scripts during
development.

Two possible alternatives:

 - Is there a reason you would not want to "install" whatever Perl
   modules you want to "use" via GITPERLLIB mechanism to
   ../../perl/blib/lib?

If we are making modifications to Git/Mediawiki.pm or even git-mw.perl
/ git-remote-mediawiki.perl, installing them without proper testing
beforehand seems wrong.

   Perhaps it will interfere with the real
   installation step in ../../perl/Makefile?

I don't think so, but I am not an expert on what's going on in
../../perl/Makefile.

   If that is the case,
   then it is not a good idea, but otherwise, that would let you use
   ../../bin-wrappers/git as-is.

 - Perhaps we could do:

        GITPERLLIB="${GPLEXTRA+$GPLEXTRA:}@@BUILD_DIR@@/perl/blib/lib"

   in wrap-for-bin.sh, so that your instruction can become

        GPLEXTRA=$(pwd) ../../bin-wrappers/git whatever-mw-thing

   and you do not have to have this file?  We would also need to
   "unset GPLEXTRA" at the beginning of test-lib.sh if we were to do
   this.

How does a developer (or user) use this script?  From your Makefile
(e.g. "make test")?  Manually following some written instruction?
In either case, the latter option feels a lot more sensible
alternative without having to maintain this extra copy of wrap-for-bin
that can easily go out of sync.

A developer uses it like any bin-wrapper : bin-wrapper/git mw preview
for instance. He can add it to its path while working on the scripts
etc ...

The best way to do it would be to factorise those new use cases
(adding new elements in $GITPERLLIB and $PATH) in the code that
generate bin-wrappers from wrap-for-bin.sh I think.

But it was weird to work on that in this serie which initial goal was
to add a preview tool for git-remote-mediawiki and ended up adding a
new perl package, a new 'git mw' command which handles subcommands,
...

In the end, this patch could be removed from the serie since it is
self-contained : it is not used by 3/5, 4/5, and 5/5.

Benoit