Re: [PATCH v6 2/5] git-remote-mediawiki: new git bin-wrapper for developement
Benoît Person benoit.per...@ensimag.fr writes: Another idea crossed my mind: for now the test suite creates a symlink of git-remote-mediawiki in the toplevel if it's not installed. It would be better to use the bin-wrapper in the testsuite I think ? Absolutely. The symlink was a dirty hack waiting for a better solution, it even appears in untracked files in git status. This bin-wrapper should be the better solution hopefully. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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
Re: [PATCH v6 2/5] git-remote-mediawiki: new git bin-wrapper for developement
GIT_ROOT_DIR=../../.. GIT_EXEC_PATH=$(cd $(dirname $0) cd ${GIT_ROOT_DIR} pwd) GIT_MEDIAWIKI=$GIT_EXEC_PATH/contrib/mw-to-git PATH=$GIT_MEDIAWIKI/contrib/mw-to-git:$PATH GPLEXTRA=$GIT_MEDIAWIKI/contrib/mw-to-git exec ${GIT_EXEC_PATH}/bin-wrappers/git $@ Should I do that in a reroll ? 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. That's not installing (i.e. not make install), just a copy within the source tree. Same as what's done in Git's toplevel. Oh right, sorry, read it too quickly :/ Well, I still find it weird to copy untested files out of contrib/mw-to-git/ though. Another idea crossed my mind: for now the test suite creates a symlink of git-remote-mediawiki in the toplevel if it's not installed. It would be better to use the bin-wrapper in the testsuite I think ? (plus, with the current solution, it may have problems finding the latest Git::Mediawiki.pm) -- 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
Re: [PATCH v6 2/5] git-remote-mediawiki: new git bin-wrapper for developement
benoit.per...@ensimag.fr writes: diff --git a/contrib/mw-to-git/bin-wrapper/git b/contrib/mw-to-git/bin-wrapper/git new file mode 100755 index 000..aa714a5 --- /dev/null +++ b/contrib/mw-to-git/bin-wrapper/git @@ -0,0 +1,27 @@ +#!/bin/sh + +# git executable wrapper script for Git-Mediawiki to run tests without +# installing all the scripts and perl packages. + +# based on $GIT_ROOT_DIR/wrap-for-bin.sh 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 $@ -- 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
Re: [PATCH v6 2/5] git-remote-mediawiki: new git bin-wrapper for developement
Junio C Hamano gits...@pobox.com writes: 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. Not only. You also need to have contrib/mw-to-git/ in the $PATH to be able to run git mw and git clone mediawiki:: - Is there a reason you would not want to install whatever Perl modules you want to use via GITPERLLIB mechanism to ../../perl/blib/lib? A previous version of the patch did something like this. This previous iterations had issues, IIRC with the PATH, but they should be solvable (and most likely independant from the $GITPERLLIB). 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. I do not like very much the idea of having the same target directory for two Makefiles, but that's just instinctive suspicion, not a real argument. - 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 You'd have to tweak the $PATH too, but that could be done by a very small wrapper calling Git's bin-wrapper/git, like this: GIT_ROOT_DIR=../../.. GIT_EXEC_PATH=$(cd $(dirname $0) cd ${GIT_ROOT_DIR} pwd) GIT_MEDIAWIKI=$GIT_EXEC_PATH/contrib/mw-to-git PATH=$GIT_MEDIAWIKI/contrib/mw-to-git:$PATH GPLEXTRA=$GIT_MEDIAWIKI/contrib/mw-to-git exec ${GIT_EXEC_PATH}/bin-wrappers/git $@ -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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
Re: [PATCH v6 2/5] git-remote-mediawiki: new git bin-wrapper for developement
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 -- 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