* [PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly
@ 2011-09-21 6:33 Johannes Sixt
2011-09-21 7:38 ` Dmitry Ivankov
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Sixt @ 2011-09-21 6:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, msysGit
From: Johannes Sixt <j6t@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.
We do not change S_ISGID from the value 0 because it is used in path.c
(via FORCE_DIR_SET_GID) to set the mode on directories in a manner that
is not supported on Windows, and 0 is the right value in this case.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
compat/mingw.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/compat/mingw.h b/compat/mingw.h
index 547568b..e2c89d6 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -22,7 +22,7 @@ typedef int socklen_t;
#define S_IWOTH 0
#define S_IXOTH 0
#define S_IRWXO (S_IROTH | S_IWOTH | S_IXOTH)
-#define S_ISUID 0
+#define S_ISUID 04000
#define S_ISGID 0
#define S_ISVTX 0
--
1.7.7.rc2.1135.gf321f
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly
2011-09-21 6:33 [PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly Johannes Sixt
@ 2011-09-21 7:38 ` Dmitry Ivankov
2011-09-21 12:07 ` Junio C Hamano
2011-09-21 12:08 ` Junio C Hamano
0 siblings, 2 replies; 6+ messages in thread
From: Dmitry Ivankov @ 2011-09-21 7:38 UTC (permalink / raw)
To: git
Johannes Sixt <j.sixt <at> viscovery.net> writes:
>
> 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.
>
> We do not change S_ISGID from the value 0 because it is used in path.c
> (via FORCE_DIR_SET_GID) to set the mode on directories in a manner that
> is not supported on Windows, and 0 is the right value in this case.
>
> Signed-off-by: Johannes Sixt <j6t <at> kdbg.org>
> ---
> compat/mingw.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 547568b..e2c89d6 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -22,7 +22,7 @@ typedef int socklen_t;
> #define S_IWOTH 0
> #define S_IXOTH 0
> #define S_IRWXO (S_IROTH | S_IWOTH | S_IXOTH)
> -#define S_ISUID 0
> +#define S_ISUID 04000
> #define S_ISGID 0
> #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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly
2011-09-21 7:38 ` Dmitry Ivankov
@ 2011-09-21 12:07 ` Junio C Hamano
2011-09-21 12:08 ` Junio C Hamano
1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2011-09-21 12:07 UTC (permalink / raw)
To: Dmitry Ivankov; +Cc: git, Johannes Sixt
Dmitry Ivankov <divanorama@gmail.com> writes:
> Johannes Sixt <j.sixt <at> viscovery.net> writes:
>>
>> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly
2011-09-21 7:38 ` Dmitry Ivankov
2011-09-21 12:07 ` Junio C Hamano
@ 2011-09-21 12:08 ` Junio C Hamano
2011-09-21 13:44 ` Dmitry Ivankov
1 sibling, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2011-09-21 12:08 UTC (permalink / raw)
To: Dmitry Ivankov; +Cc: git, Johannes Sixt
Dmitry Ivankov <divanorama@gmail.com> writes:
> Johannes Sixt <j.sixt <at> viscovery.net> writes:
>>
>> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly
2011-09-21 12:08 ` Junio C Hamano
@ 2011-09-21 13:44 ` Dmitry Ivankov
2011-09-21 15:13 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Ivankov @ 2011-09-21 13:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Sixt
On Wed, Sep 21, 2011 at 6:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Dmitry Ivankov <divanorama@gmail.com> writes:
>
>> Johannes Sixt <j.sixt <at> viscovery.net> writes:
>>>
>>> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly
2011-09-21 13:44 ` Dmitry Ivankov
@ 2011-09-21 15:13 ` Junio C Hamano
0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2011-09-21 15:13 UTC (permalink / raw)
To: Dmitry Ivankov; +Cc: Junio C Hamano, git, Johannes Sixt
Dmitry Ivankov <divanorama@gmail.com> writes:
>> 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.
Failed -- was too subtle to extract a hoped-for response "come to think of
it we should have done it as a separate field in the structure".
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-09-21 15:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-21 6:33 [PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly Johannes Sixt
2011-09-21 7:38 ` Dmitry Ivankov
2011-09-21 12:07 ` Junio C Hamano
2011-09-21 12:08 ` Junio C Hamano
2011-09-21 13:44 ` Dmitry Ivankov
2011-09-21 15:13 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).