{"thread":{"id":"28455","subject":"[PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly","startedAt":"2011-09-21T06:33:28Z","lastAt":"2011-09-21T15:13:31Z","messageCount":6,"participants":["Johannes Sixt","Dmitry Ivankov","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"175918","messageId":"4E798538.7070106@viscovery.net","threadId":"28455","inReplyTo":null,"subject":"[PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-09-21T06:33:28Z","receivedAt":"2011-09-21T06:33:28Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Sixt <j6t@kdbg.org>\n\n8fb3ad76 (fast-import: prevent producing bad delta) introduced the first\nuse of S_ISUID. Since before this commit the value was irrelevant, we had\nonly a dummy definition in mingw.h. But beginning with this commit the\nmacro must expand to a reasonable value. Make it so.\n\nWe do not change S_ISGID from the value 0 because it is used in path.c\n(via FORCE_DIR_SET_GID) to set the mode on directories in a manner that\nis not supported on Windows, and 0 is the right value in this case.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n compat/mingw.h |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 547568b..e2c89d6 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -22,7 +22,7 @@ typedef int socklen_t;\n #define S_IWOTH 0\n #define S_IXOTH 0\n #define S_IRWXO (S_IROTH | S_IWOTH | S_IXOTH)\n-#define S_ISUID 0\n+#define S_ISUID 04000\n #define S_ISGID 0\n #define S_ISVTX 0\n\n-- \n1.7.7.rc2.1135.gf321f\n"},{"id":"175920","messageId":"loom.20110921T092135-714@post.gmane.org","threadId":"28455","inReplyTo":"4E798538.7070106@viscovery.net","subject":"Re: [PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-09-21T07:38:18Z","receivedAt":"2011-09-21T07:38:18Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"Johannes Sixt <j.sixt <at> viscovery.net> writes:\n\n> \n> From: Johannes Sixt <j6t <at> kdbg.org>\n> \n> 8fb3ad76 (fast-import: prevent producing bad delta) introduced the first\n> use of S_ISUID. Since before this commit the value was irrelevant, we had\n> only a dummy definition in mingw.h. But beginning with this commit the\n> macro must expand to a reasonable value. Make it so.\n> \n> We do not change S_ISGID from the value 0 because it is used in path.c\n> (via FORCE_DIR_SET_GID) to set the mode on directories in a manner that\n> is not supported on Windows, and 0 is the right value in this case.\n> \n> Signed-off-by: Johannes Sixt <j6t <at> kdbg.org>\n> ---\n>  compat/mingw.h |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n> \n> diff --git a/compat/mingw.h b/compat/mingw.h\n> index 547568b..e2c89d6 100644\n> --- a/compat/mingw.h\n> +++ b/compat/mingw.h\n> @@ -22,7 +22,7 @@ typedef int socklen_t;\n>  #define S_IWOTH 0\n>  #define S_IXOTH 0\n>  #define S_IRWXO (S_IROTH | S_IWOTH | S_IXOTH)\n> -#define S_ISUID 0\n> +#define S_ISUID 04000\n>  #define S_ISGID 0\n>  #define S_ISVTX 0\n> \nOw, it's awkward that the issue was discussed in [1] but slipped and nobody \nnoticed, especially me being a patch sender.\n\nIf we choose patch from [1] I'd also change a comment to smth like\n/* \n * We abuse the 04000 bit on directories to mean \"do not delta\".\n * It is a S_ISUID bit on setuid platforms and an unused bit on\n * non-setuid platforms supported in git. In either case git ignores\n * the bit, so it's safe to abuse it locally.\n */\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/179223/focus=179762\n"},{"id":"175938","messageId":"7v39fq3xpc.fsf@alter.siamese.dyndns.org","threadId":"28455","inReplyTo":"loom.20110921T092135-714@post.gmane.org","subject":"Re: [PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-21T12:07:59Z","receivedAt":"2011-09-21T12:07:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dmitry Ivankov <divanorama@gmail.com> writes:\n\n> Johannes Sixt <j.sixt <at> viscovery.net> writes:\n>> \n>> From: Johannes Sixt <j6t <at> kdbg.org>\n>> \n>> 8fb3ad76 (fast-import: prevent producing bad delta) introduced the first\n>> use of S_ISUID. Since before this commit the value was irrelevant, we had\n>> only a dummy definition in mingw.h. But beginning with this commit the\n>> macro must expand to a reasonable value. Make it so.\n>>  #define S_ISVTX 0\n>> ...\n> Ow, it's awkward that the issue was discussed in [1] but slipped and nobody \n> noticed, especially me being a patch sender.\n>\n> If we choose patch from [1] I'd also change a comment to smth like\n> /* \n>  * We abuse the 04000 bit on directories to mean \"do not delta\".\n>  * It is a S_ISUID bit on setuid platforms and an unused bit on\n>  * non-setuid platforms supported in git. In either case git ignores\n>  * the bit, so it's safe to abuse it locally.\n>  */\n>\n> [1] http://thread.gmane.org/gmane.comp.version-control.git/179223/focus=179762\n\nI think that the fix from Jonathan to stop abusing S_ISUID is much more\npreferrable; the Windows platform shouldn't have to worry about this.\n\nAnd it would be even better to use a value that does not overlap with the\nusual bits for do-not-delta bit if possible.\n"},{"id":"175940","messageId":"7vfwjq2hoo.fsf@alter.siamese.dyndns.org","threadId":"28455","inReplyTo":"loom.20110921T092135-714@post.gmane.org","subject":"Re: [PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-21T12:08:03Z","receivedAt":"2011-09-21T12:08:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dmitry Ivankov <divanorama@gmail.com> writes:\n\n> Johannes Sixt <j.sixt <at> viscovery.net> writes:\n>> \n>> From: Johannes Sixt <j6t <at> kdbg.org>\n>> \n>> 8fb3ad76 (fast-import: prevent producing bad delta) introduced the first\n>> use of S_ISUID. Since before this commit the value was irrelevant, we had\n>> only a dummy definition in mingw.h. But beginning with this commit the\n>> macro must expand to a reasonable value. Make it so.\n>>  #define S_ISVTX 0\n>> ...\n> Ow, it's awkward that the issue was discussed in [1] but slipped and nobody \n> noticed, especially me being a patch sender.\n>\n> If we choose patch from [1] I'd also change a comment to smth like\n> /* \n>  * We abuse the 04000 bit on directories to mean \"do not delta\".\n>  * It is a S_ISUID bit on setuid platforms and an unused bit on\n>  * non-setuid platforms supported in git. In either case git ignores\n>  * the bit, so it's safe to abuse it locally.\n>  */\n>\n> [1] http://thread.gmane.org/gmane.comp.version-control.git/179223/focus=179762\n\nI think that the fix from Jonathan to stop abusing S_ISUID is much more\npreferrable; the Windows platform shouldn't have to worry about this.\n\nAnd it would be even better to use a value that does not overlap with the\nusual bits for do-not-delta bit if possible.\n"},{"id":"175947","messageId":"CA+gfSn8VaTLcrLqb3HGJpjL5WJMHZz4kPPUtyeHPCsdmO5iU8g@mail.gmail.com","threadId":"28455","inReplyTo":"7vfwjq2hoo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-09-21T13:44:55Z","receivedAt":"2011-09-21T13:44:55Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"On Wed, Sep 21, 2011 at 6:08 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Dmitry Ivankov <divanorama@gmail.com> writes:\n>\n>> Johannes Sixt <j.sixt <at> viscovery.net> writes:\n>>>\n>>> From: Johannes Sixt <j6t <at> kdbg.org>\n>>>\n>>> 8fb3ad76 (fast-import: prevent producing bad delta) introduced the first\n>>> use of S_ISUID. Since before this commit the value was irrelevant, we had\n>>> only a dummy definition in mingw.h. But beginning with this commit the\n>>> macro must expand to a reasonable value. Make it so.\n>>>  #define S_ISVTX 0\n>>> ...\n>> Ow, it's awkward that the issue was discussed in [1] but slipped and nobody\n>> noticed, especially me being a patch sender.\n>>\n>> If we choose patch from [1] I'd also change a comment to smth like\n>> /*\n>>  * We abuse the 04000 bit on directories to mean \"do not delta\".\n>>  * It is a S_ISUID bit on setuid platforms and an unused bit on\n>>  * non-setuid platforms supported in git. In either case git ignores\n>>  * the bit, so it's safe to abuse it locally.\n>>  */\n>>\n>> [1] http://thread.gmane.org/gmane.comp.version-control.git/179223/focus=179762\n>\n> I think that the fix from Jonathan to stop abusing S_ISUID is much more\n> preferrable; the Windows platform shouldn't have to worry about this.\n>\n> And it would be even better to use a value that does not overlap with the\n> usual bits for do-not-delta bit if possible.\n\nDepends on what is a usual bit. I'll use linux defines for mode bits.\n\nThere are S_ISVTX, S_ISUID completely unused in git, S_ISGID is used somehow.\n9 lower rwx bits are used as well as S_IFREG and S_IFDIR. Remaining are S_IFIFO\n(used somehow), S_IFCHR (part of GITLNK).\n\nS_ISUID in fast-import input stream isn't accepted, so the only danger\nis it may come\nfrom a tree object (but not yet, git-fsck doesn't allow it). Or if\nthere appears a platform\nwith different S_I{F,S}* definitions, which will break more git parts\nthan just fast-import.\n\nI remember there was a thread concerning platform vs git-core mode\nbits. I'd just use\nhard-coded 04000 bit in fast-import as a hot-fix and leave the rest\nfor the bits topic.\nWith S_ISUID in a comment near 04000 it'll be a grep-able hard-coded\nconstant, so\nit should be ok.\n"},{"id":"175950","messageId":"7v39fq2ajo.fsf@alter.siamese.dyndns.org","threadId":"28455","inReplyTo":"CA+gfSn8VaTLcrLqb3HGJpjL5WJMHZz4kPPUtyeHPCsdmO5iU8g@mail.gmail.com","subject":"Re: [PATCH di/fast-import-deltified-tree] Windows: define S_ISUID properly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-21T15:13:31Z","receivedAt":"2011-09-21T15:13:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dmitry Ivankov <divanorama@gmail.com> writes:\n\n>> And it would be even better to use a value that does not overlap with the\n>> usual bits for do-not-delta bit if possible.\n>\n> Depends on what is a usual bit. I'll use linux defines for mode bits.\n\nFailed -- was too subtle to extract a hoped-for response \"come to think of\nit we should have done it as a separate field in the structure\".\n"}]}