threads / patch / 3295

patchDon't propagate SKIPPED warning to future commits.

Subject: [PATCH] Don't propagate SKIPPED warning to future commits.

## tl;dr

3 messages between Feb 11, 2006 and Feb 11, 2006. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Carl Worth· Feb 11, 2006, 01:41 UTC · lore

Here's a simple bug fix for the (currently undocumented) -S flag to git-cvsimport.

The -S flag allows an import to proceed in the face of a missing ,v file in the CVS repository. It marks this by adding a SKIPPED warning in the relevant commit.

Without this patch, the SKIPPED warning propagates to all future commits, (and accumulates with future SKIPPED warnings).

Martin did the original work on -S to fix an import of the cairo repository.

However, I just discovered that a correct fix for the cairo import problem was for me to just "rm -rf ~/.cvsps". (I suppose calling with a -x option would have worked too.)

I don't know how common it is for CVS repositories to change in ways such that the cvsps cache gets totally confused as in my case, but it might be worth adding something to cvsimport to help this. Some options could be:

 * Use cvsps -x always, to ignore the cache.

 * Warn the user, (perhaps emit some description of stale cache
   possibility if the import fails?).

 * Add a note to the documentation.

Signed-off-by: Carl Worth <cworth@cworth.org>

---

 git-cvsimport.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

c816d743a1faf65fb209273a471b15c7b51b55df

Show changes to git-cvsimport.perl +1 −0

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 6b63aa2..25db0ed 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -749,6 +749,7 @@ my $commit = sub {
 	if (@skipped) {
 	    $logmsg .= "\n\n\nSKIPPED:\n\t";
 	    $logmsg .= join("\n\t", @skipped) . "\n";
+	    @skipped = ();
 	}
 
 	print $pw "$logmsg\n"

-- 
1.1.6.g9da5

Junio C Hamano· Feb 11, 2006, 02:21 UTC · re: Carl Worth · lore

Re: [PATCH] Don't propagate SKIPPED warning to future commits.

Carl Worth <cworth@cworth.org> writes:

Show 9 quoted lines

> Here's a simple bug fix for the (currently undocumented) -S flag to
> git-cvsimport.
>
> The -S flag allows an import to proceed in the face of a missing ,v
> file in the CVS repository. It marks this by adding a SKIPPED warning
> in the relevant commit.
>
> Without this patch, the SKIPPED warning propagates to all future
> commits, (and accumulates with future SKIPPED warnings).

I might be missing something fundamental, but isn't prapagating the warning a good thing? If an earlier commit is suspected to lack some path that needs to be there (I think that is what "missing ,v file" means), wouldn't later commits built on top of that one be by definition missing that path?

Carl Worth· Feb 11, 2006, 14:47 UTC · re: Junio C Hamano · lore

Re: [PATCH] Don't propagate SKIPPED warning to future commits.

On Fri, 10 Feb 2006 18:21:30 -0800, Junio C Hamano wrote:

> 
> I might be missing something fundamental, but isn't propagating
> the warning a good thing?

Yes, that would be fine.

The problem with the current behavior is that the warnings accumulate for every commit that affects the particular file. So, after import, the head commit will have a commit message ending in:

	SKIPPED: broken-file
	SKIPPED: broken-file
	SKIPPED: broken-file
	...

With one line per affected commit. So an alternate patch that simply kept the warning unique per file would be fine too.

The discussion may be entirely moot since I don't know when it is reasonable for a file to be missing in this way. In my specific case I think the missing ,v file happened due to a combination of factors, including old data in ~/.cvsps, and that data being stale due to some manual surgery on the ,v files, (copying or renaming or so).

It would probably be fine to leave the current behavior until someone runs into this again.

-Carl