"David D. Kilzer" <ddkilzer@xxxxxxxxxx> wrote:
> Implement "git-svn info" for files and directories based on the "svn info"
> command. Note that the -r/--revision argument is not supported yet.
>
> Added 18 tests in t/t9117-git-svn-info.sh.
>
> Signed-off-by: David D. Kilzer <ddkilzer@xxxxxxxxxx>
> ---
>
> Looking for feedback on this patch. Specifically, I'm looking for insight
> for the two FIXME comments in the cmd_info() function added to git-svn.
> (I can't help but think I'm missing a plumbing command or a basic concept.
> Pointers to code, web pages or man pages are welcome.)
>
> Note that I've tried to cover all the bases that "svn info" does (by using
> tests), except supporting the -r/--revision argument.
Hi David,
Wow. I can honestly say I've never even noticed the "Schedule:" field
in `svn info'. I would've been perfectly happy to accept an
implementation of `git svn info' without that :)
> I've also tried to be aggressive in extracting common code into functions.
I like it, but this should be a separate patch.
> +sub canonicalize_path {
> + my ($path) = @_;
> + my $dotSlashAdded = 0;
camelCase variables requires more time for the brain to parse (they're
easier to write, but take more time to read), please use snake_case like
the rest of git-svn (and git).
> +sub cmd_info {
> + my $path = canonicalize_path(shift or ".");
> + unless (scalar(@_) == 0) {
> + die "Too many arguments specified\n";
> + }
> +
> + # FIXME: We use a combination of git-diff, git-ls-files and git-cat-file
> + # to divine the state and type of object that was passed in as $path.
> + # There has to be a better way. Note that only $diffStatus is used
> + # beyond setting $isDirectory below.
I agree it's pretty ugly. You can probably expand git-runstatus to do
this. git-commit.sh used to use something like this until git-runstatus
was added. On the other hand, I'd be content if we dropped support
for this info entirely since `git-status' is perfectly good.
IMNSHO, "URL:" and "Repository Root:" and occasionally "Revision:" (on the
top-level directory) would be the only useful things this command would
have to offer.
Being able to run something like `git svn info --url <path>'
to get something like http://svn.foo.org/project/trunk/<path> would be
nice, too.
> +
> + my $diffStatus = (split(' ', command_oneline(qw(diff --cached --name-status --), $path) || ""))[0] || "";
Please wrap lines at 80 characters. I have a hard time following long
lines.
> + my $checksum;
> + # FIXME: We fail to generate the correct checksum for deleted
> + # symlinks here. How do we know if a deleted file was a symlin
git ls-tree HEAD <filename> will show the mode of a deleted file
> @@ -0,0 +1,236 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2007 David D. Kilzer
> +
> +test_description='git-svn info'
> +
> +. ./lib-git-svn.sh
> +
> +test_expect_success 'setup repository and import' "
> + rm -rf info gitwc svnwc &&
All git tests should start you off on a clean trash/ directory...
> + mkdir info &&
> + cd info &&
> + echo one > file &&
> + ln -s file symlink-file &&
> + mkdir directory &&
> + touch directory/.placeholder &&
> + ln -s directory symlink-directory &&
> + svn import -m 'initial' . $svnrepo &&
> + cd .. &&
> + mkdir gitwc &&
> + cd gitwc &&
> + git-svn init $svnrepo &&
> + git-svn fetch &&
> + cd .. &&
> + svn co $svnrepo svnwc &&
> + touch -c -r svnwc/file gitwc/file &&
> + touch -c -r svnwc/directory gitwc/directory &&
> + touch -c -r svnwc/symlink-file gitwc/symlink-file &&
> + touch -c -r svnwc/symlink-directory gitwc/symlink-directory
Are -r and -c portable? I remember writing test-chmtime to workaround
some arguments for touch not being portable.
> +test_expect_success 'info no arguments' "
> + (cd svnwc; svn info) > expected.info-no-arguments &&
> + (cd gitwc; git-svn info) > actual.info-no-arguments &&
> + diff -u expected.info-no-arguments actual.info-no-arguments
> + "
Can we expect the output of "svn info" to not change between
versions? I know "svn status" has changed between versions of
svn. I'd prefer if we keep the expected.* files hard-coded
in a test directory and compare those instead. Maybe use sed
to substitute placeholders for timestamps..
Also, git-diff can be used against arbitrary files nowadays, no
need to rely on a working diff command in the system :)
> +test_expect_success 'info added-file' "
> + echo two > gitwc/added-file &&
> + cd gitwc &&
> + git add added-file &&
> + cd .. &&
> + cp -p gitwc/added-file svnwc/added-file &&
I can't remember if cp -p is portable, either...
--
Eric Wong
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
