Re: [PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:52:05

Dmitry Ivankov [off-list ref] writes:
Johannes Sixt <j.sixt <at> viscovery.net> writes:
quoted
From: Johannes Sixt <j6t <at> kdbg.org>

8fb3ad76 (fast-import: prevent producing bad delta) introduced the first
use of S_ISUID. Since before this commit the value was irrelevant, we had
only a dummy definition in mingw.h. But beginning with this commit the
macro must expand to a reasonable value. Make it so.
 #define S_ISVTX 0
...
Ow, it's awkward that the issue was discussed in [1] but slipped and nobody 
noticed, especially me being a patch sender.

If we choose patch from [1] I'd also change a comment to smth like
/* 
 * We abuse the 04000 bit on directories to mean "do not delta".
 * It is a S_ISUID bit on setuid platforms and an unused bit on
 * non-setuid platforms supported in git. In either case git ignores
 * the bit, so it's safe to abuse it locally.
 */

[1] http://thread.gmane.org/gmane.comp.version-control.git/179223/focus=179762
I think that the fix from Jonathan to stop abusing S_ISUID is much more
preferrable; the Windows platform shouldn't have to worry about this.

And it would be even better to use a value that does not overlap with the
usual bits for do-not-delta bit if possible.

Re: [PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly

From: Dmitry Ivankov <hidden>
Date: 2016-06-15 22:52:05

On Wed, Sep 21, 2011 at 6:08 PM, Junio C Hamano [off-list ref] wrote:
Dmitry Ivankov [off-list ref] writes:
quoted
Johannes Sixt <j.sixt <at> viscovery.net> writes:
quoted
From: Johannes Sixt <j6t <at> kdbg.org>

8fb3ad76 (fast-import: prevent producing bad delta) introduced the first
use of S_ISUID. Since before this commit the value was irrelevant, we had
only a dummy definition in mingw.h. But beginning with this commit the
macro must expand to a reasonable value. Make it so.
 #define S_ISVTX 0
...
Ow, it's awkward that the issue was discussed in [1] but slipped and nobody
noticed, especially me being a patch sender.

If we choose patch from [1] I'd also change a comment to smth like
/*
 * We abuse the 04000 bit on directories to mean "do not delta".
 * It is a S_ISUID bit on setuid platforms and an unused bit on
 * non-setuid platforms supported in git. In either case git ignores
 * the bit, so it's safe to abuse it locally.
 */

[1] http://thread.gmane.org/gmane.comp.version-control.git/179223/focus=179762
I think that the fix from Jonathan to stop abusing S_ISUID is much more
preferrable; the Windows platform shouldn't have to worry about this.

And it would be even better to use a value that does not overlap with the
usual bits for do-not-delta bit if possible.
Depends on what is a usual bit. I'll use linux defines for mode bits.

There are S_ISVTX, S_ISUID completely unused in git, S_ISGID is used somehow.
9 lower rwx bits are used as well as S_IFREG and S_IFDIR. Remaining are S_IFIFO
(used somehow), S_IFCHR (part of GITLNK).

S_ISUID in fast-import input stream isn't accepted, so the only danger
is it may come
from a tree object (but not yet, git-fsck doesn't allow it). Or if
there appears a platform
with different S_I{F,S}* definitions, which will break more git parts
than just fast-import.

I remember there was a thread concerning platform vs git-core mode
bits. I'd just use
hard-coded 04000 bit in fast-import as a hot-fix and leave the rest
for the bits topic.
With S_ISUID in a comment near 04000 it'll be a grep-able hard-coded
constant, so
it should be ok.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help