threads / patch / 11585

patchgit-relink.perl: warn() instead of die() on directory open failure

Subject: [PATCH] git-relink.perl: warn() instead of die() on directory open failure

## tl;dr

3 messages between Jan 11, 2008 and Jan 12, 2008. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Brandon Casey· Jan 11, 2008, 18:11 UTC · lore

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---

Sometimes the repository to link to is not under your control. If it contains files or unreadable directories, git-relink will die without this patch.

-brandon

 git-relink.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to git-relink.perl +1 −1

diff --git a/git-relink.perl b/git-relink.perl
index f6b4f6a..391b12d 100755
--- a/git-relink.perl
+++ b/git-relink.perl
@@ -69,7 +69,7 @@ sub do_scan_directory($$$) {
 	my $dfulldir = sprintf("%sobjects/%s/",$dstdir,$subdir);
 
 	opendir(S,$sfulldir)
-		or die "Failed to opendir $sfulldir: $!";
+		or (warn "Failed to opendir $sfulldir: $!" and return);
 
 	foreach my $file (grep(!/\.{1,2}$/, readdir(S))) {
 		my $sfilename = $sfulldir . $file;

-- 
1.5.4.rc2.84.gf85fd-dirty

Junio C Hamano· Jan 12, 2008, 02:14 UTC · re: Brandon Casey · lore

Re: [PATCH] git-relink.perl: warn() instead of die() on directory open failure

Brandon Casey <casey@nrlssc.navy.mil> writes:

Show 7 quoted lines

> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> ---
>
>
> Sometimes the repository to link to is not under your control.
> If it contains files or unreadable directories, git-relink will
> die without this patch.

I am not so sure if dying is a bad behaviour, if it is because you are trying to link against an object store that you may not be able to read. I actually think we should actively refuse to, in order to prevent future problems. After seeing the command die, you will talk to the owner of that "master" object store and ask him to fix permissions (or he may choose to say "please do not share with me").

However, if the "master" object store has pruned its loose objects since you read @hashdirs out of it, that opendir() can fail, and we should silently ignore the whole $subdir.

Brandon Casey· Jan 12, 2008, 02:26 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-relink.perl: warn() instead of die() on directory open failure

Junio C Hamano wrote:

Show 17 quoted lines

> Brandon Casey <casey@nrlssc.navy.mil> writes:
> 
>> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
>> ---
>>
>>
>> Sometimes the repository to link to is not under your control.
>> If it contains files or unreadable directories, git-relink will
>> die without this patch.
> 
> I am not so sure if dying is a bad behaviour, if it is because
> you are trying to link against an object store that you may not
> be able to read.  I actually think we should actively refuse to,
> in order to prevent future problems.  After seeing the command
> die, you will talk to the owner of that "master" object store
> and ask him to fix permissions (or he may choose to say "please
> do not share with me").

The case for me was that the objects directory contained temporary pack files. This is a perfectly valid state for a git repository, but relink() currently aborts the whole effort when it encounters a non-directory.

Stale tmp packs can remain if the user aborted a git command before it was finished. (hmm, maybe git-gc --prune could remove these too?)

I only mentioned the unreadable directory case as another possibility. You're probably right about that one.

-brandon