Re: cvs import and internal plumbing commands

Antony Stubbs [off-list ref] writes:

On 21/07/2009, at 3:34 AM, Jeff King wrote:
...

quoted

But I guess the plan was softened, and we have no deprecation plan for
dashed forms in exec-path. So maybe my thinking was outdated.

quoted

So the preferred fix might be just the matter of adding one line

	$ENV{'PATH'} = `git --exec-path`. ":$ENV{'PATH'}";

at the beginning of the script.

I can see that for a totally third-party script which wanted to use
git
plumbing.  But why do that for a git-* script? The "git" wrapper
sets up
the environment like that already.

-Peff

[note: do not top post]

But regardless, surely calling straight into git is simpler/nicer and
more likely to just work (as on OSX)?

From the 1.6.0 release notes
(http://www.kernel.org/pub/software/scm/git/docs/RelNotes-1.6.0.txt ):
"""With the default Makefile settings, most of the programs are now
installed outside your $PATH, except for "git", "gitk" and some server
side programs that need to be accessible for technical reasons.
Invoking a git subcommand as "git-xyzzy" from the command line has
been deprecated since early 2006 (and officially announced in 1.5.4
release notes); use of them from your scripts after adding output from
"git --exec-path" to the $PATH is still supported in this release, but
users are again strongly encouraged to adjust their scripts to use
"git xyzzy" form, as we will stop installing "git-xyzzy" hardlinks for
built-in commands in later releases. """

In case you are too new to the git land, I'd point out that "... as we
will stop installing" part was rescinded after a big fiasco that
immediately followed 1.6.0 release.

note the "but users are again strongly encouraged to adjust their
scripts to use "git xyzzy" form". Why not just apply the patch and be
done with it?

Note that my suggestion on $PATH=$(git --exec-path):$PATH was a response
to Jeff's message, which was the same message your patch was response to.
In other words, our messages crossed, and I do not have an issue with
fixing this either way.  If your patch fixes the issue correctly that
would be great.

Has anybody reviewed the submitted patch?  I do not use cvsimport these
days anymore for anything real, so the only assurance I have on your patch
so far are my eyeballs and whatever tests done in test scripts in the t/
directory.  Independent "Yeah, it looks good and it works for me" would
make me feel a bit safer.

On Tue, Jul 21, 2009 at 05:43:08PM -0700, Junio C Hamano wrote:

Note that my suggestion on $PATH=$(git --exec-path):$PATH was a response
to Jeff's message, which was the same message your patch was response to.
In other words, our messages crossed, and I do not have an issue with
fixing this either way.  If your patch fixes the issue correctly that
would be great.

I am still confused about what the "issue" is. That is, why this is a
bugfix for Anthony instead of simply a code modernization. IOW, I think
there is something else broken about his install if cvsimport cannot
find the dashed plumbing commands, and we may simply be papering over
it.

That being said, I think the code modernization is of value itself, so
let's do it.

Has anybody reviewed the submitted patch?  I do not use cvsimport these
days anymore for anything real, so the only assurance I have on your patch
so far are my eyeballs and whatever tests done in test scripts in the t/
directory.  Independent "Yeah, it looks good and it works for me" would
make me feel a bit safer.

I am lucky enough not to have needed git-cvsimport for some time, so I
cannot say "it works for me." But I have read the patch; I didn't see
any errors in the conversion, and assuming the conversion is done
correctly, the behavior should be otherwise identical. So

Acked-by: Jeff King <redacted>

-Peff