{"thread":{"id":"1984","subject":"Destructive side-effect of \"cg-status\"","startedAt":"2005-09-30T16:03:53Z","lastAt":"2005-10-03T18:32:04Z","messageCount":18,"participants":["Wolfgang Denk","Martin Langhoff","Linus Torvalds","Junio C Hamano","H. Peter Anvin","Matthias Urlichs"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"9558","messageId":"20050930160353.F025C352B7B@atlas.denx.de","threadId":"1984","inReplyTo":null,"subject":"Destructive side-effect of \"cg-status\"","fromName":"Wolfgang Denk","fromEmail":"wd@denx.de","sentAt":"2005-09-30T16:03:53Z","receivedAt":"2005-09-30T16:03:53Z","isPatch":false,"sender":{"key":"wd@denx.de","avatar":null},"body":"So far I thought \"cg-status\" is a harmless command which just\ndisplays some status information. It ain't so. One of our engineers\nreported a corrupted repository after I ran \"cg-status\" in his\ndirectory:\n\n$ cg-status\nHeads:\n >master 805f93e4ca96d0c0cb2d2f9532d9666b22961e88\n R origin 805f93e4ca96d0c0cb2d2f9532d9666b22961e88\n\nerror: open failed\nfatal: cache corrupted\nerror: open failed\n? COPYING\n? CREDITS\n? Documentation/00-INDEX\n? Documentation/BUG-HUNTING\n...\nerror: open failed\nread_cache: Permission denied\n...\nerror: open failed\nread_cache: Permission denied\n...\n\n\nAs mentioned before, all I did was running \"cg-status\" in his\ndirectory. Here is what happens:\n\nBefore:\n\n\t-> rpm -q cogito\n\tcogito-0.15.1-1\n\t-> id\n\tuid=500(wd) gid=500(wd) groups=200(gitmaster),400(denx),500(wd)\n\t-> umask\n\t0002\n\t-> ls -ld .git\n\tdrwxrwxrwx 6 sr sr 80 Sep 30 17:49 .git\n\t-> ls -l .git/index\n\t-rw-r--r-- 1 sr sr 1728032 Sep 30 17:17 .git/index\n\nThen:\n\n\t-> cg-status\n\tHeads:\n\t >master 805f93e4ca96d0c0cb2d2f9532d9666b22961e88\n\t R origin 805f93e4ca96d0c0cb2d2f9532d9666b22961e88\n\n\tM arch/ppc/configs/bubinga_defconfig\n\tM arch/ppc/configs/walnut_defconfig\n\t-> ls -l .git/index\n\t-rw------- 1 wd wd 1728032 Sep 30 17:49 .git/index\n\t^^^^^^^^^^ ^^^^^\n\nThat means, that \"cg-status\" actually *rewrote* .git/index, with me\n(wd) as new owner, and - ignoring my umask - with permissions that\nprevent the original owner (sr) to access the file!\n\nArghhhh!!!\n\nBest regards,\n\nWolfgang Denk\n\n-- \nSoftware Engineering: Embedded and Realtime Systems, Embedded Linux\nPhone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de\nGenerally speaking, there are other ways to accomplish whatever it is\nthat you think you need ... - Doug Gwyn\n"},{"id":"9589","messageId":"46a038f90510010324h65bea422tf9a519a014ed4844@mail.gmail.com","threadId":"1984","inReplyTo":"20050930160353.F025C352B7B@atlas.denx.de","subject":"Re: Destructive side-effect of \"cg-status\"","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2005-10-01T10:24:54Z","receivedAt":"2005-10-01T10:24:54Z","isPatch":false,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 10/1/05, Wolfgang Denk wrote:\n> So far I thought \"cg-status\" is a harmless command which just\n> displays some status information. It ain't so. One of our engineers\n> reported a corrupted repository after I ran \"cg-status\" in his\n> directory:\n\nInteresting... cg-status, and sometimes cg-diff, have to update the\nindex. The index is the trick behind git's performance (and some other\nsmarts). It's never really touched by Cogito, but by the git-*-index\ncommands.\n\nPerhaps git-*-index commands should check ownership vs current uid and\nprint a warning?\n\ncheers,\n\n\nmartin\n"},{"id":"9591","messageId":"Pine.LNX.4.64.0510010934290.3378@g5.osdl.org","threadId":"1984","inReplyTo":"20050930160353.F025C352B7B@atlas.denx.de","subject":"Re: Destructive side-effect of \"cg-status\"","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-10-01T16:41:41Z","receivedAt":"2005-10-01T16:41:41Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 30 Sep 2005, Wolfgang Denk wrote:\n>\n> So far I thought \"cg-status\" is a harmless command which just\n> displays some status information. It ain't so. One of our engineers\n> reported a corrupted repository after I ran \"cg-status\" in his\n> directory:\n\nWell, it's not corrupted, but yes, the index file ends up unreadable.\n\n> That means, that \"cg-status\" actually *rewrote* .git/index, with me\n> (wd) as new owner, and - ignoring my umask - with permissions that\n> prevent the original owner (sr) to access the file!\n\nThe umask thing looks like a bug. Fixed thus.\n\nAlso, arguably we should try to avoid writing the index file when not \nnecessary, although the fact is, that cg-status (and \"git status\") _do_ \nneed to actually keep it up-to-date in order to do the right thing. Also \ntrue of some other programs that might otherwise appear to be read-only \n(ie I've considered doing the same thing for \"git diff\").\n\nWe used to have that optimization, but it was broken. I fixed it but \ndisabled it for fear of other bugs.\n\nBut honoring umask would seem to be a no-brainer.\n\n\t\tLinus\n\n----\ndiff --git a/index.c b/index.c\n--- a/index.c\n+++ b/index.c\n@@ -29,7 +29,7 @@ int hold_index_file_for_update(struct ca\n \t\tsignal(SIGINT, remove_lock_file_on_signal);\n \t\tatexit(remove_lock_file);\n \t}\n-\treturn open(cf->lockfile, O_RDWR | O_CREAT | O_EXCL, 0600);\n+\treturn open(cf->lockfile, O_RDWR | O_CREAT | O_EXCL, 0666);\n }\n \n int commit_index_file(struct cache_file *cf)\n"},{"id":"9592","messageId":"7vr7b53y0n.fsf@assigned-by-dhcp.cox.net","threadId":"1984","inReplyTo":"Pine.LNX.4.64.0510010934290.3378@g5.osdl.org","subject":"Re: Destructive side-effect of \"cg-status\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-10-01T18:14:32Z","receivedAt":"2005-10-01T18:14:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> -\treturn open(cf->lockfile, O_RDWR | O_CREAT | O_EXCL, 0600);\n> +\treturn open(cf->lockfile, O_RDWR | O_CREAT | O_EXCL, 0666);\n\nGood spotting - thanks. We tried to use 0666/0777 everywhere\nand let umask do its job, but this was one of the two places we\nstill had 0[67]00. I'd do the same for the other 0600 in\nmailsplit.c, not that I think it matters, but just for\nconsistency.\n\nUnrelated to the topic at hand, but related to the mode bits --\ntar-tree generates archives with 0644/0755 permission bits. It\nmight not be a bad idea to just let the tar command honor umask\nof the extracter, by storing 0666 and 0777 in the archive.\n\nI always work in an environment where umask 002 is the norm, and\nget irritated when upstream tarballs of other peoples' projects\ncreate directories with mode 0755, making me do chmod 2775 on\nthem.\n"},{"id":"9594","messageId":"7vk6gx3vkt.fsf_-_@assigned-by-dhcp.cox.net","threadId":"1984","inReplyTo":"7vr7b53y0n.fsf@assigned-by-dhcp.cox.net","subject":"Honor extractor's umask in git-tar-tree.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-10-01T19:07:14Z","receivedAt":"2005-10-01T19:07:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The archive generated with git-tar-tree had 0755 and 0644 mode bits.\nThis inconvenienced the extractor with umask 002 by robbing g+w bit\nunconditionally. Just write it out with loose permissions bits and\nlet the umask of the extractor do its job.\n\nSigned-off-by: Junio C Hamano \n---\nJunio C Hamano writes:\n\n> Unrelated to the topic at hand, but related to the mode bits --\n> tar-tree generates archives with 0644/0755 permission bits. It\n> might not be a bad idea to just let the tar command honor umask\n> of the extracter, by storing 0666 and 0777 in the archive.\n>\n> I always work in an environment where umask 002 is the norm, and\n> get irritated when upstream tarballs of other peoples' projects\n> create directories with mode 0755, making me do chmod 2775 on\n> them.\n\ndiff --git a/tar-tree.c b/tar-tree.c\n--- a/tar-tree.c\n+++ b/tar-tree.c\n@@ -353,6 +353,7 @@ static void traverse_tree(void *buffer, \n \n \t\tif (size < namelen + 20 || sscanf(buffer, \"%o\", &mode) != 1)\n \t\t\tdie(\"corrupt 'tree' file\");\n+\t\tmode |= (mode & 0100) ? 0777 : 0666;\n \t\tbuffer = sha1 + 20;\n \t\tsize -= namelen + 20;\n \n"},{"id":"9595","messageId":"20051001194216.EE3E5353D8E@atlas.denx.de","threadId":"1984","inReplyTo":"Pine.LNX.4.64.0510010934290.3378@g5.osdl.org","subject":"Re: Destructive side-effect of \"cg-status\"","fromName":"Wolfgang Denk","fromEmail":"wd@denx.de","sentAt":"2005-10-01T19:42:16Z","receivedAt":"2005-10-01T19:42:16Z","isPatch":false,"sender":{"key":"wd@denx.de","avatar":null},"body":"In message \nLinus Torvalds wrote:\n> \n> Also, arguably we should try to avoid writing the index file when not \n> necessary, although the fact is, that cg-status (and \"git status\") _do_ \n> need to actually keep it up-to-date in order to do the right thing. Also \n> true of some other programs that might otherwise appear to be read-only \n> (ie I've considered doing the same thing for \"git diff\").\n\nBut shouldn't it be possible to run such commands as \"status\" and\n\"diff\" in a repository for which I have only read permissions? Or how\ncan I find out about the status of another user's repository without\nactually modifying it?\n\nAlso, error reporting is IMHO not sufficient and misleading. For\nexample:\n\n\t$ git status 2>&1 | less\n\terror: open failed\n\t#\n\t# Updated but not checked in:\n\t# (will commit)\n\t#\n\t# deleted: CHANGELOG\n\t# deleted: COPYING\n\t# deleted: CREDITS\n\t# deleted: MAINTAINERS\n\t# deleted: MAKEALL\n\t# deleted: Makefile\n\t# deleted: README\n\t...\n\t[all files in the repository flagged as \"deleted\" !]\n\t#\n\terror: open failed\n\tread_cache: Permission denied\n\nThe \"error: open failed\" should at leats include the file name and\nthe errno/strerror message. Same for the \"read_cache: Permission\ndenied\" - of course, if you knot the git internals you will know what\nthis means, but the average user has no idea that he should check the\npermissions of .git/index.\n\nFinally, a thick fat warning should be added to the documentation\nthat these commands actually (may) modify the repository. This was\ntotally unexpected for me.\n\nThanks.\n\nBest regards,\n\nWolfgang Denk\n\n-- \nSoftware Engineering: Embedded and Realtime Systems, Embedded Linux\nPhone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de\nIn an infinite universe all things are possible, including the possi-\nbility that the universe does not exist.\n - Terry Pratchett, _The Dark Side of the Sun_\n"},{"id":"9596","messageId":"Pine.LNX.4.64.0510011303560.3378@g5.osdl.org","threadId":"1984","inReplyTo":"20051001194216.EE3E5353D8E@atlas.denx.de","subject":"Re: Destructive side-effect of \"cg-status\"","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-10-01T20:24:27Z","receivedAt":"2005-10-01T20:24:27Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 1 Oct 2005, Wolfgang Denk wrote:\n> \n> The \"error: open failed\" should at leats include the file name and\n> the errno/strerror message. Same for the \"read_cache: Permission\n> denied\" - of course, if you knot the git internals you will know what\n> this means, but the average user has no idea that he should check the\n> permissions of .git/index.\n\nAgreed.\n\nSomething like this would seem sane.\n\n\t\tLinus\n\n---\nSubject: Better error reporting for \"git status\"\n\nInstead of \"git status\" ignoring (and hiding) potential errors from the \n\"git-update-index\" call, make it exit if it fails, and show the error.\n\nIn order to do this, use the \"-q\" flag (to ignore not-up-to-date files) \nand add a new \"--unmerged\" flag that allows unmerged entries in the index\nwithout any errors.\n\nThis also avoids marking the index \"changed\" if an entry isn't actually \nmodified, and makes sure that we exit with an understandable error message \nif the index is corrupt or unreadable. \"read_cache()\" no longer returns an \nerror for the caller to check.\n\nFinally, make die() and usage() exit with recognizable error codes, if we\never want to check the failure reason in scripts.\n\nSigned-off-by: Linus Torvalds \n---\ndiff --git a/git-status.sh b/git-status.sh\n--- a/git-status.sh\n+++ b/git-status.sh\n@@ -37,7 +37,7 @@ refs/heads/master) ;;\n *)\techo \"# On branch $branch\" ;;\n esac\n \n-git-update-index --refresh >/dev/null 2>&1\n+git-update-index -q --unmerged --refresh || exit\n \n if test -f \"$GIT_DIR/HEAD\"\n then\ndiff --git a/read-cache.c b/read-cache.c\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -464,11 +464,15 @@ int read_cache(void)\n \n \terrno = EBUSY;\n \tif (active_cache)\n-\t\treturn error(\"more than one cachefile\");\n+\t\treturn active_nr;\n+\n \terrno = ENOENT;\n \tfd = open(get_index_file(), O_RDONLY);\n-\tif (fd < 0)\n-\t\treturn (errno == ENOENT) ? 0 : error(\"open failed\");\n+\tif (fd < 0) {\n+\t\tif (errno == ENOENT)\n+\t\t\treturn 0;\n+\t\tdie(\"index file open failed (%s)\", strerror(errno));\n+\t}\n \n \tsize = 0; // avoid gcc warning\n \tmap = MAP_FAILED;\n@@ -480,7 +484,7 @@ int read_cache(void)\n \t}\n \tclose(fd);\n \tif (map == MAP_FAILED)\n-\t\treturn error(\"mmap failed\");\n+\t\tdie(\"index file mmap failed (%s)\", strerror(errno));\n \n \thdr = map;\n \tif (verify_hdr(hdr, size) < 0)\n@@ -501,7 +505,7 @@ int read_cache(void)\n unmap:\n \tmunmap(map, size);\n \terrno = EINVAL;\n-\treturn error(\"verify header failed\");\n+\tdie(\"index file corrupt\");\n }\n \n #define WRITE_BUFFER_SIZE 8192\ndiff --git a/update-index.c b/update-index.c\n--- a/update-index.c\n+++ b/update-index.c\n@@ -13,7 +13,7 @@\n * like \"git-update-index *\" and suddenly having all the object\n * files be revision controlled.\n */\n-static int allow_add = 0, allow_remove = 0, allow_replace = 0, not_new = 0, quiet = 0, info_only = 0;\n+static int allow_add = 0, allow_remove = 0, allow_replace = 0, allow_unmerged = 0, not_new = 0, quiet = 0, info_only = 0;\n static int force_remove;\n \n /* Three functions to allow overloaded pointer return; see linux/err.h */\n@@ -135,7 +135,7 @@ static struct cache_entry *refresh_entry\n \n \tchanged = ce_match_stat(ce, &st);\n \tif (!changed)\n-\t\treturn ce;\n+\t\treturn NULL;\n \n \tif (ce_modified(ce, &st))\n \t\treturn ERR_PTR(-EINVAL);\n@@ -156,16 +156,20 @@ static int refresh_cache(void)\n \t\tstruct cache_entry *ce, *new;\n \t\tce = active_cache[i];\n \t\tif (ce_stage(ce)) {\n-\t\t\tprintf(\"%s: needs merge\\n\", ce->name);\n-\t\t\thas_errors = 1;\n \t\t\twhile ((i < active_nr) &&\n \t\t\t ! strcmp(active_cache[i]->name, ce->name))\n \t\t\t\ti++;\n \t\t\ti--;\n+\t\t\tif (allow_unmerged)\n+\t\t\t\tcontinue;\n+\t\t\tprintf(\"%s: needs merge\\n\", ce->name);\n+\t\t\thas_errors = 1;\n \t\t\tcontinue;\n \t\t}\n \n \t\tnew = refresh_entry(ce);\n+\t\tif (!new)\n+\t\t\tcontinue;\n \t\tif (IS_ERR(new)) {\n \t\t\tif (not_new && PTR_ERR(new) == -ENOENT)\n \t\t\t\tcontinue;\n@@ -335,6 +339,10 @@ int main(int argc, const char **argv)\n \t\t\t\tallow_remove = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(path, \"--unmerged\")) {\n+\t\t\t\tallow_unmerged = 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(path, \"--refresh\")) {\n \t\t\t\thas_errors |= refresh_cache();\n \t\t\t\tcontinue;\ndiff --git a/usage.c b/usage.c\n--- a/usage.c\n+++ b/usage.c\n@@ -15,7 +15,7 @@ static void report(const char *prefix, c\n void usage(const char *err)\n {\n \tfprintf(stderr, \"usage: %s\\n\", err);\n-\texit(1);\n+\texit(129);\n }\n \n void die(const char *err, ...)\n@@ -25,7 +25,7 @@ void die(const char *err, ...)\n \tva_start(params, err);\n \treport(\"fatal: \", err, params);\n \tva_end(params);\n-\texit(1);\n+\texit(128);\n }\n \n int error(const char *err, ...)\n"},{"id":"9599","messageId":"433F52DC.5090906@zytor.com","threadId":"1984","inReplyTo":"7vk6gx3vkt.fsf_-_@assigned-by-dhcp.cox.net","subject":"Re: Honor extractor's umask in git-tar-tree.","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-10-02T03:24:12Z","receivedAt":"2005-10-02T03:24:12Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Junio C Hamano wrote:\n> The archive generated with git-tar-tree had 0755 and 0644 mode bits.\n> This inconvenienced the extractor with umask 002 by robbing g+w bit\n> unconditionally. Just write it out with loose permissions bits and\n> let the umask of the extractor do its job.\n\nI've thought that it would be nice if the files/directories were written \ninto the archive with 0666/0777 permissions by default, and then \nextracted with the umask honoured. A special option then could be used \nto add files with special permissions, like files in .ssh, which *have* \nto be g-w or sshd will reject them.\n\n\t-hpa\n"},{"id":"9602","messageId":"pan.2005.10.02.09.55.52.564046@smurf.noris.de","threadId":"1984","inReplyTo":"433F52DC.5090906@zytor.com","subject":"Re: Honor extractor's umask in git-tar-tree.","fromName":"Matthias Urlichs","fromEmail":"smurf@smurf.noris.de","sentAt":"2005-10-02T09:55:55Z","receivedAt":"2005-10-02T09:55:55Z","isPatch":false,"sender":{"key":"matthias@urlichs.de","avatar":"https://gravatar.com/avatar/2708905af227313eba6f2b2ae0f7d0259b5ac5d71baef58fe5a13c699ce0bbf0?d=mp&s=160"},"body":"Hi, H. Peter Anvin wrote:\n\n> I've thought that it would be nice if the files/directories were written\n> into the archive with 0666/0777 permissions by default, and then\n> extracted with the umask honoured.\n\nThe git archive oesn't *have* permissions, just one \"execute\" bit.\n\n> A special option then could be used\n> to add files with special permissions, like files in .ssh, which *have*\n> to be g-w or sshd will reject them.\n> \nI'd include a script in the archive which you'd run afterwards to fix\nproblems like this. IMHO, in most situations you'll need it anyway\n(for instance, to re-start services).\n\n-- \nMatthias Urlichs | {M:U} IT Design @ m-u-it.de | smurf@smurf.noris.de\nDisclaimer: The quote was selected randomly. Really. | http://smurf.noris.de\n - -\nAs I approached the intersection a sign suddenly appeared in a place\nwhere no stop sign had ever appeared before. I was unable to stop in\ntime to avoid the accident. To avoid hitting the bumper of the car in\nfront, I struck the pedestrian.\n"},{"id":"9623","messageId":"4340B73B.1090409@zytor.com","threadId":"1984","inReplyTo":"pan.2005.10.02.09.55.52.564046@smurf.noris.de","subject":"Re: Honor extractor's umask in git-tar-tree.","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-10-03T04:44:43Z","receivedAt":"2005-10-03T04:44:43Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Matthias Urlichs wrote:\n> Hi, H. Peter Anvin wrote:\n> \n>>I've thought that it would be nice if the files/directories were written\n>>into the archive with 0666/0777 permissions by default, and then\n>>extracted with the umask honoured.\n> \n> The git archive oesn't *have* permissions, just one \"execute\" bit.\n> \n\nMy point is that I believe it should. It has the bitfield for it, it \njust doesn't use it at the moment.\n\n>> A special option then could be used\n>>to add files with special permissions, like files in .ssh, which *have*\n>>to be g-w or sshd will reject them.\n> \n> I'd include a script in the archive which you'd run afterwards to fix\n> problems like this. IMHO, in most situations you'll need it anyway\n> (for instance, to re-start services).\n\nThat is true in some cases, but I highly disagree with the statement \"most\".\n\n\t-hpa\n"},{"id":"9624","messageId":"7virwfuqwv.fsf@assigned-by-dhcp.cox.net","threadId":"1984","inReplyTo":"4340B73B.1090409@zytor.com","subject":"Re: Honor extractor's umask in git-tar-tree.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-10-03T05:10:24Z","receivedAt":"2005-10-03T05:10:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"H. Peter Anvin\" writes:\n\n> My point is that I believe it should. It has the bitfield for it, it \n> just doesn't use it at the moment.\n\nIt is a bit more complicated than that.\n\nLong time ago, we used to store the full permission bits and\nended up storing files in 0644 and 0664 modes, depending on who\nis writing the tree object. People with umask 022 checked out\nfrom a tree that recorded blobs with 0664 bits and ended up\ngetting \"mode changed\" diff all the time, which was unacceptable\nfrom the SCM point of view. We _could_ have really changed the\nmode bits representation in the tree objects back then to have\ntype + executable bit, but to preserve backward compatibility,\nwe chose to keep the bitfield layout and changed the code to\ntreat 1006xx and 1007yy in older trees to be equivalent to\n100644 and 100755. These days, for newly written tree objects,\nabove xx and yy 6-bit fields are \"Must Be 4\" and \"Must Be 5\"\nfields, respectively, not bitfields to store arbitrary group and\nother permission information. git-fsck-objects even complains\nabout them.\n\nSo in that sense, it does _not_ have the bitfield for it, and\nobviously we cannot use what we do not have.\n"},{"id":"9630","messageId":"43415C9A.1090502@zytor.com","threadId":"1984","inReplyTo":"7virwfuqwv.fsf@assigned-by-dhcp.cox.net","subject":"Re: Honor extractor's umask in git-tar-tree.","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-10-03T16:30:18Z","receivedAt":"2005-10-03T16:30:18Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Junio C Hamano wrote:\n> \"H. Peter Anvin\" writes:\n> \n> \n>>My point is that I believe it should. It has the bitfield for it, it \n>>just doesn't use it at the moment.\n> \n> \n> It is a bit more complicated than that.\n> \n> Long time ago, we used to store the full permission bits and\n> ended up storing files in 0644 and 0664 modes, depending on who\n> is writing the tree object. People with umask 022 checked out\n> from a tree that recorded blobs with 0664 bits and ended up\n> getting \"mode changed\" diff all the time, which was unacceptable\n> from the SCM point of view. We _could_ have really changed the\n> mode bits representation in the tree objects back then to have\n> type + executable bit, but to preserve backward compatibility,\n> we chose to keep the bitfield layout and changed the code to\n> treat 1006xx and 1007yy in older trees to be equivalent to\n> 100644 and 100755. These days, for newly written tree objects,\n> above xx and yy 6-bit fields are \"Must Be 4\" and \"Must Be 5\"\n> fields, respectively, not bitfields to store arbitrary group and\n> other permission information. git-fsck-objects even complains\n> about them.\n> \n> So in that sense, it does _not_ have the bitfield for it, and\n> obviously we cannot use what we do not have.\n> \n\nWelcome to the wonderful world of evolving file formats.\n\nAs you stated above, we currently use this field in a very inefficient \nmanner, because of old mistakes. There are several ways to recover from \nhere, some of which are more complex than others.\n\nIn the case of git, there isn't just the requirement to maintain old \nformats indefinitely (due to the cryptographic chain), but also that new \nobjects that are compatible with old format should be written in the old \nformat to maintain the aliasing properties. These are obstacles that \nare perfectly possible to overcome, although it takes a bit of legwork.\n\nIf the old-format (with random write bits) is out of circulation -- \nwhich I can't tell for sure they it is, but Linus' kernel tree doesn't \nseem to have any of these objects -- then the answer is very simple: \nredefine this field _a posteori_ to be the mode ^ 022 (or perhaps more \nsanely,\nmode ^ (mode & 0200 ? 022 : 0)). Compatibility and contents is fully \npreserved. No problem.\n\nIf there are still old-format trees in circulation and compatibility \nwith these very old trees need to be maintained, then it's a bit more \ncomplicated, but literally just a bit. This data is already stored in \ntext form in the object store, so there aren't any funnies with \nexpanding it. For example, encode a leading + on the octal value if \nthis is a value with \"I really mean it\" permissions (and *only* those \nvalues.) This is even readable by older versions of git, since they \nwill just blindly ignore the + sign.\n\n\t-hpa\n"},{"id":"9632","messageId":"7v8xxasenp.fsf@assigned-by-dhcp.cox.net","threadId":"1984","inReplyTo":"43415C9A.1090502@zytor.com","subject":"Re: Honor extractor's umask in git-tar-tree.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-10-03T17:18:02Z","receivedAt":"2005-10-03T17:18:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"H. Peter Anvin\" writes:\n\n> As you stated above, we currently use this field in a very inefficient \n> manner, because of old mistakes. There are several ways to recover from \n> here, some of which are more complex than others.\n\nSolution for in-tree permission mode bits you outlined looked\nfine (I'll have to re-read the part about \"mode xor (mode &\n022)...\" part later, though).\n\nFor in-cache permission mode bits, we would probably need\nsomething like this:\n\n * git-update-index will pick up the filesystem bits with the\n current semantics (i.e. look only at (mode & 0100) and\n force 0644 or 0755) by default; --full-perm-bits option\n would bypass this bits munging.\n\n Once a file is added with --full-perm-bits, it might be\n nice if index file remembers to pick up the full bits next\n time git-update-index is run on the path. This could be\n achieved by saying that anything stored in the cache with\n non 100644, 100755 nor 120000 bits are such paths without\n having to change the index file format.\n\n * there are bunch of codes that assume 0644 and 0755 are the\n norm but also know that there are ancient trees that have\n 0664 and 0775 and try to treat them equivalently. They need\n to be selectively neutered; this applies to in-tree\n permission bits as well.\n\n git-read-tree will read permission mode bits from tree\n object as-is. I.e. you will get 0644 and 0755 in cache from\n the existing tree objects. When you check things out with\n 002 umask, you will get 0664 and 0775 on the filesystem. We\n do not want to consider this \"mode changed by the user\".\n\n git-update-index --refresh code should not be mode neutered\n to prevent this. The same thing goes for diff. These\n currently canonicalize mode bits by looking at (mode &\n 0100), but should be changed to do so only when index has\n already the canonical mode bits, or something like that.\n\n * git-write-tree and git-fsck-objects probably has code to\n reject and correct abnormal mode bits. They need to be\n neutered.\n"},{"id":"9633","messageId":"43416A27.7070302@zytor.com","threadId":"1984","inReplyTo":"7v8xxasenp.fsf@assigned-by-dhcp.cox.net","subject":"Re: Honor extractor's umask in git-tar-tree.","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-10-03T17:28:07Z","receivedAt":"2005-10-03T17:28:07Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Junio C Hamano wrote:\n> \n> For in-cache permission mode bits, we would probably need\n> something like this:\n> \n> * git-update-index will pick up the filesystem bits with the\n> current semantics (i.e. look only at (mode & 0100) and\n> force 0644 or 0755) by default; --full-perm-bits option\n> would bypass this bits munging.\n> \n> Once a file is added with --full-perm-bits, it might be\n> nice if index file remembers to pick up the full bits next\n> time git-update-index is run on the path. This could be\n> achieved by saying that anything stored in the cache with\n> non 100644, 100755 nor 120000 bits are such paths without\n> having to change the index file format.\n> \n\nOne could also say that since oddball permissions are an exception, not \nthe rule, that one should use a \"git-chmod\" command to enter the \npermissions in the cache. The correct answer is probably *both* that \nand --full-permissions (or whatever) since they both probably apply to \ndifferent workflows.\n\n\t-hpa\n"},{"id":"9634","messageId":"Pine.LNX.4.64.0510031028370.31407@g5.osdl.org","threadId":"1984","inReplyTo":"43415C9A.1090502@zytor.com","subject":"Re: Honor extractor's umask in git-tar-tree.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-10-03T17:45:46Z","receivedAt":"2005-10-03T17:45:46Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 3 Oct 2005, H. Peter Anvin wrote:\n> \n> If the old-format (with random write bits) is out of circulation -- which I\n> can't tell for sure they it is, but Linus' kernel tree doesn't seem to have\n> any of these objects\n\nOh, it does.\n\nRun \"git-fsck-cache --full --strict\", and you'll get several trees that \nthe strict checker marks as bad. I think it's mostly all one entry, namely \narch/i386/kernel/vsyscall-note.S being marked 0664.\n\nGit itself has even more of them - the kernel actually has fewer, because \nmost work was done with a consistent umask of 022 (ie mostly mine), and by \nthe time others started using git actively, we'd already changed the git \nrules.\n\nHowever, there's nothing that says that we couldn't use one more bit in \nthe \"mode\" flag to just say \"this is an _exact_ mode, please preserve it\". \nA kind of \"sticky mode\" for git. We've got _bits_ plenty: it's an ASCII \ntext-mode representation in the trees (infinite bits), and even in the \nindex it's a 32-bit thing that we only use 12 bits of (9 bits for \npermissions, 3 bits for the sparsely represented directory/symlink/regular \nfile)\n\nWe'd have to be a bit careful to preserve that bit when doing an index \nrefresh, but it's really not very hard. The hardest part is actually doing \nso for directories, since we don't keep the directories in the index at \n_all_.\n\nBut the fact is, it wouldn't solve the git-tar-tree thing. We can \n_represent_ exact masks, but we don't _want_ to, because normally it just \nleads to horrible problems with different people having different umasks. \nSo in order to avoid having mode change merges, we'd _still_ have to make \nthe current \"0666/0777 + umask\" be the normal one, and you'd use this \n\"exact mode\" thing only for very special cases (ie for backing up your \nhome directory or similar, _not: for a distributed SCM).\n\nAs to tar: I think the current\n\n if (S_ISDIR(mode) || S_ISREG(mode))\n mode |= (mode & 0100) ? 0777 : 0666;\n\nis wrong. It makes things world-writable by default, and that's just \ndangerous. \"tar\" normally won't apply umask when untarring (there's a flag \nfor it, but I have never ever used it myself, and I doubt anybody else \nreally does either - it's called \"--no-same-permissions\" in GNU tar).\n\nI think a \"0775\" or \"0664\" might be acceptable (an umask of 002 is at \nleast _normal_), but I suspect 0755/0644 is really better. Doing a simple \n\n\tchmod -R +w\n\nafterwards is better (and takes umask into account) than a \"chmod -R o-w\", \nsince the latter leaves the tree writable for a while.\n\nIe default permissions are better off being too strict than too lax. Basic \nsecurity.\n\nOf course, if we were to add the \"exact mode\" bit, then git-tar-tree \nshould obviously honor that for any files that have that bit set.\n\n\t\tLinus\n"},{"id":"9635","messageId":"434172FD.7020302@zytor.com","threadId":"1984","inReplyTo":"Pine.LNX.4.64.0510031028370.31407@g5.osdl.org","subject":"Re: Honor extractor's umask in git-tar-tree.","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-10-03T18:05:49Z","receivedAt":"2005-10-03T18:05:49Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Linus Torvalds wrote:\n> \n> As to tar: I think the current\n> \n> if (S_ISDIR(mode) || S_ISREG(mode))\n> mode |= (mode & 0100) ? 0777 : 0666;\n> \n> is wrong. It makes things world-writable by default, and that's just \n> dangerous.\n\nThat's standard in the Unix world, though; of course, the user's umask \nshouldn't be set to zero unless things are in very special \ncircumstances. In the case of tar, the umask is applied on extraction \nunless the user explicitly specifies -p.\n\n\t-hpa\n"},{"id":"9636","messageId":"Pine.LNX.4.64.0510031113500.31407@g5.osdl.org","threadId":"1984","inReplyTo":"434172FD.7020302@zytor.com","subject":"Re: Honor extractor's umask in git-tar-tree.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-10-03T18:18:15Z","receivedAt":"2005-10-03T18:18:15Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 3 Oct 2005, H. Peter Anvin wrote:\n>\n> Linus Torvalds wrote:\n> > \n> > As to tar: I think the current\n> > \n> > if (S_ISDIR(mode) || S_ISREG(mode))\n> > mode |= (mode & 0100) ? 0777 : 0666;\n> > \n> > is wrong. It makes things world-writable by default, and that's just\n> > dangerous.\n> \n> That's standard in the Unix world, though; of course, the user's umask\n> shouldn't be set to zero unless things are in very special circumstances. In\n> the case of tar, the umask is applied on extraction unless the user explicitly\n> specifies -p.\n\nIs it? The only place umask is mentioned in the man-page is\n\n --no-same-permissions\n apply user?s umask when extracting files instead of recorded\n permissions\n\nbut if tar really does honor umask, then hey, that 0777/0666 is fine.\n\n(Ahh, googling a bit more, it appears that \"-p\" is default for root, which \nexplains why you'd need the \"anti-flag\").\n\n\t\tLinus\n"},{"id":"9637","messageId":"43417924.7000308@zytor.com","threadId":"1984","inReplyTo":"Pine.LNX.4.64.0510031113500.31407@g5.osdl.org","subject":"Re: Honor extractor's umask in git-tar-tree.","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2005-10-03T18:32:04Z","receivedAt":"2005-10-03T18:32:04Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"Linus Torvalds wrote:\n> \n> Is it? The only place umask is mentioned in the man-page is\n> \n> --no-same-permissions\n> apply user?s umask when extracting files instead of recorded\n> permissions\n> \n> but if tar really does honor umask, then hey, that 0777/0666 is fine.\n> \n> (Ahh, googling a bit more, it appears that \"-p\" is default for root, which \n> explains why you'd need the \"anti-flag\").\n> \n\nYeah, that assymetry is rather unfortunate.\n\n\t-hpa\n"}]}