{"thread":{"id":"7621","subject":"[ANNOUNCE] GIT 1.5.1.1","startedAt":"2007-04-12T02:09:34Z","lastAt":"2007-04-22T19:29:55Z","messageCount":15,"participants":["Junio C Hamano","Linus Torvalds","Nicolas Pitre"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"39189","messageId":"7vhcrml4wx.fsf@assigned-by-dhcp.cox.net","threadId":"7621","inReplyTo":null,"subject":"[ANNOUNCE] GIT 1.5.1.1","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-12T02:09:34Z","receivedAt":"2007-04-12T02:09:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The latest maintenance release GIT 1.5.1.1 is available at the\nusual places:\n\n  http://www.kernel.org/pub/software/scm/git/\n\n  git-1.5.1.1.tar.{gz,bz2}\t\t\t(tarball)\n  git-htmldocs-1.5.1.1.tar.{gz,bz2}\t\t(preformatted docs)\n  git-manpages-1.5.1.1.tar.{gz,bz2}\t\t(preformatted docs)\n  RPMS/$arch/git-*-1.5.1.1-1.$arch.rpm\t(RPM)\n\nGIT v1.5.1.1 Release Notes\n==========================\n\nFixes since v1.5.1\n------------------\n\n* Documentation updates\n\n  - The --left-right option of rev-list and friends is documented.\n\n  - The documentation for cvsimport has been majorly improved.\n\n  - \"git-show-ref --exclude-existing\" was documented.\n\n* Bugfixes\n\n  - The implementation of -p option in \"git cvsexportcommit\" had\n    the meaning of -C (context reduction) option wrong, and\n    loosened the context requirements when it was told to be\n    strict.\n\n  - \"git cvsserver\" did not behave like the real cvsserver when\n    client side removed a file from the working tree without\n    doing anything else on the path.  In such a case, it should\n    restore it from the checked out revision.\n\n  - \"git fsck\" issued an alarming error message on detached\n    HEAD.  It is not an error since at least 1.5.0.\n\n  - \"git send-email\" produced of References header of unbounded length;\n    fixed this with line-folding.\n\n  - \"git archive\" to download from remote site should not\n    require you to be in a git repository, but it incorrectly\n    did.\n\n  - \"git apply\" ignored -p<n> for \"diff --git\" formatted\n    patches.\n\n  - \"git rerere\" recorded a conflict that had one side empty\n    (the other side adds) incorrectly; this made merging in the\n    other direction fail to use previously recorded resolution.\n\n  - t4200 test was broken where \"wc -l\" pads its output with\n    spaces.\n\n  - \"git branch -m old new\" to rename branch did not work\n    without a configuration file in \".git/config\".\n\n  - The sample hook for notification e-mail was misnamed.\n\n  - gitweb did not show type-changing patch correctly in the\n    blobdiff view.\n\n  - git-svn did not error out with incorrect command line options.\n\n  - git-svn fell into an infinite loop when insanely long commit\n    message was found.\n\n  - git-svn dcommit and rebase was confused by patches that were\n    merged from another branch that is managed by git-svn.\n\n----------------------------------------------------------------\n\nChanges since v1.5.1 are as follows:\n\nArjen Laarhoven (4):\n      usermanual.txt: some capitalization nits\n      t3200-branch.sh: small language nit\n      t5300-pack-object.sh: portability issue using /usr/bin/stat\n      Makefile: iconv() on Darwin has the old interface\n\nBrian Gernhardt (3):\n      Fix t4200-rerere for white-space from \"wc -l\"\n      Document --left-right option to rev-list.\n      Distinguish branches by more than case in tests.\n\nDana How (1):\n      Fix lseek(2) calls with args 2 and 3 swapped\n\nEric Wong (3):\n      git-svn: bail out on incorrect command-line options\n      git-svn: dcommit/rebase confused by patches with git-svn-id: lines\n      git-svn: fix log command to avoid infinite loop on long commit messages\n\nFrank Lichtenheld (7):\n      cvsimport: sync usage lines with existing options\n      cvsimport: Improve documentation of CVSROOT and CVS module determination\n      cvsimport: Improve usage error reporting\n      cvsimport: Reorder options in documentation for better understanding\n      cvsimport: Improve formating consistency\n      cvsserver: small corrections to asciidoc documentation\n      cvsserver: Fix handling of diappeared files on update\n\nGeert Bosch (1):\n      Fix renaming branch without config file\n\nGerrit Pape (1):\n      rename contrib/hooks/post-receieve-email to contrib/hooks/post-receive-email.\n\nJakub Narebski (1):\n      gitweb: Fix bug in \"blobdiff\" view for split (e.g. file to symlink) patches\n\nJim Meyering (1):\n      (encode_85, decode_85): Mark source buffer pointer as \"const\".\n\nJulian Phillips (1):\n      Documentation: show-ref: document --exclude-existing\n\nJunio C Hamano (7):\n      rerere: make sorting really stable.\n      Fix dependency of common-cmds.h\n      Documentation: tighten dependency for git.{html,txt}\n      Prepare for 1.5.1.1\n      Add Documentation/cmd-list.made to .gitignore\n      fsck: do not complain on detached HEAD.\n      GIT 1.5.1.1\n\nLars Hjemli (2):\n      rename_ref(): only print a warning when config-file update fails\n      Make builtin-branch.c handle the git config file\n\nRené Scharfe (1):\n      Revert \"builtin-archive: use RUN_SETUP\"\n\nShawn O. Pearce (1):\n      Honor -p<n> when applying git diffs\n\nTomash Brechko (1):\n      cvsexportcommit -p : fix the usage of git-apply -C.\n\nVille Skyttä (1):\n      DESTDIR support for git/contrib/emacs\n\nYOSHIFUJI Hideaki (1):\n      Avoid composing too long \"References\" header.\n"},{"id":"40096","messageId":"7v1widrl0o.fsf@assigned-by-dhcp.cox.net","threadId":"7621","inReplyTo":"7vhcrml4wx.fsf@assigned-by-dhcp.cox.net","subject":"[ANNOUNCE] GIT 1.5.1.2","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-22T06:16:55Z","receivedAt":"2007-04-22T06:16:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The latest maintenance release GIT 1.5.1.2 is available at the\nusual places:\n\n  http://www.kernel.org/pub/software/scm/git/\n\n  git-1.5.1.2.tar.{gz,bz2}\t\t\t(tarball)\n  git-htmldocs-1.5.1.2.tar.{gz,bz2}\t\t(preformatted docs)\n  git-manpages-1.5.1.2.tar.{gz,bz2}\t\t(preformatted docs)\n  RPMS/$arch/git-*-1.5.1.2-1.$arch.rpm\t(RPM)\n\nGIT v1.5.1.2 Release Notes\n==========================\n\nFixes since v1.5.1.1\n--------------------\n\n* Bugfixes\n\n  - \"git clone\" over http from a repository that has lost the\n    loose refs by running \"git pack-refs\" were broken (a code to\n    deal with this was added to \"git fetch\" in v1.5.0, but it\n    was missing from \"git clone\").\n\n  - \"git diff a/ b/\" incorrectly fell in \"diff between two\n    filesystem objects\" codepath, when the user most likely\n    wanted to limit the extent of output to two tracked\n    directories.\n\n  - git-quiltimport had the same bug as we fixed for\n    git-applymbox in v1.5.1.1 -- it gave an alarming \"did not\n    have any patch\" message (but did not actually fail and was\n    harmless).\n\n  - various git-svn fixes.\n\n  - Sample update hook incorrectly always refused requests to\n    delete branches through push.\n\n  - git-blame on a very long working tree path had buffer\n    overrun problem.\n\n  - git-apply did not like to be fed two patches in a row that created\n    and then modified the same file.\n\n  - git-svn was confused when a non-project was stored directly under\n    trunk/, branches/ and tags/.\n\n  - git-svn wants the Error.pm module that was at least as new\n    as what we ship as part of git; install ours in our private\n    installation location if the one on the system is older.\n\n  - An earlier update to command line integer parameter parser was\n    botched and made 'update-index --cacheinfo' completely useless.\n\n\n* Documentation updates\n\n  - Various documentation updates from J. Bruce Fields, Frank\n    Lichtenheld, Alex Riesen and others.  Andrew Ruder started a\n    war on undocumented options.\n\n\n----------------------------------------------------------------\n\nChanges since v1.5.1.1 are as follows:\n\nAlex Riesen (3):\n      Use rev-list --reverse in git-rebase.sh\n      Document -g (--walk-reflogs) option of git-log\n      Fix overwriting of files when applying contextually independent diffs\n\nAndrew Ruder (8):\n      Update git-am documentation\n      Update git-applymbox documentation\n      Update git-apply documentation\n      Update git-annotate/git-blame documentation\n      Update git-archive documentation\n      Update git-cherry-pick documentation\n      Fix unmatched emphasis tag in git-tutorial\n      Update git-config documentation\n\nAndy Whitcroft (1):\n      fix up strtoul_ui error handling\n\nCarlos Rica (1):\n      Use const qualifier for 'sha1' parameter in delete_ref function\n\nEric Wong (4):\n      git-svn: respect lower bound of -r/--revision when following parent\n      git-svn: quiet some warnings when run only with --version/--help\n      git-svn: don't allow globs to match regular files\n      perl: install private Error.pm if the site version is older than our own\n\nEygene Ryabinkin (2):\n      Teach gitk to use the user-defined UI font everywhere.\n      Improve look-and-feel of the gitk tool.\n\nFrank Lichtenheld (5):\n      config.txt: Document gitcvs.allbinary\n      config.txt: Document core.autocrlf\n      config.txt: Change pserver to server in description of gitcvs.*\n      config.txt: Fix grammatical error in description of http.noEPSV\n      git-shortlog: Fix two formatting errors in asciidoc documentation\n\nGerrit Pape (2):\n      variable $projectdesc needs to be set before checking against unchanged default.\n      Have sample update hook not refuse deleting a branch through push.\n\nJ. Bruce Fields (7):\n      Documentation: minor edits of git-lost-found manpage\n      Documentation: clarify git-checkout -f, minor editing\n      Documentation: clarify track/no-track option.\n      user-manual: fix discussion of default clone\n      user-manual: detached HEAD\n      user-manual: start revising \"internals\" chapter\n      user-manual: use detached head when rewriting history\n\nJim Meyering (1):\n      sscanf/strtoul: parse integers robustly\n\nJunio C Hamano (4):\n      Do not default to --no-index when given two directories.\n      Start preparing for 1.5.1.2\n      git-clone: fix dumb protocol transport to clone from pack-pruned ref\n      GIT 1.5.1.2\n\nLinus Torvalds (1):\n      git-quiltimport complaining yet still working\n\nMatthias Lederhofer (1):\n      handle_options in git wrapper miscounts the options it handled.\n\nMichael Spang (1):\n      git-blame: Fix overrun in fake_working_tree_commit()\n\nSam Vilain (1):\n      git-tar-tree: complete deprecation conversion message\n\nShawn O. Pearce (1):\n      git-gui: Brown paper bag fix division by 0 in blame\n"},{"id":"40129","messageId":"alpine.LFD.0.98.0704221019100.9964@woody.linux-foundation.org","threadId":"7621","inReplyTo":"7v1widrl0o.fsf@assigned-by-dhcp.cox.net","subject":"Re: [ANNOUNCE] GIT 1.5.1.2","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-22T17:22:47Z","receivedAt":"2007-04-22T17:22:47Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 21 Apr 2007, Junio C Hamano wrote:\n>\n> The latest maintenance release GIT 1.5.1.2 is available at the\n> usual places:\n\nWell, by \"available\" you probably mean \"not available\", because it doesn't \nactually work.\n\nI get EPERM on pack-e00affefe0f779d0f9b0507aef25a1733f4a9117.idx/pack, \nbecause they are\n\n\t-r-------- 1 junio junio  1120880\n\t-r-------- 1 junio junio 15709370\n\nrespectively.\n\nAs a result, nothing really works, ie:\n\n\t[torvalds@hera git.git]$ git log\n\terror: Could not read 42c4b58059fa9af65e90f2c418bb551e30d1d32f\n\nand doing a \"git pull\" will just result in lots of\n\n\terror: refs/heads/maint does not point to a valid object!\n\terror: refs/heads/next does not point to a valid object!\n\terror: refs/heads/todo does not point to a valid object!\n\t...\n\nwhich is perhaps a bit of a misleading error message (technically true, \nbut ..)\n\nOops.\n\n\t\tLinus\n"},{"id":"40133","messageId":"7vvefonvdz.fsf@assigned-by-dhcp.cox.net","threadId":"7621","inReplyTo":"alpine.LFD.0.98.0704221019100.9964@woody.linux-foundation.org","subject":"Re: [ANNOUNCE] GIT 1.5.1.2","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-22T17:58:48Z","receivedAt":"2007-04-22T17:58:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Sat, 21 Apr 2007, Junio C Hamano wrote:\n>>\n>> The latest maintenance release GIT 1.5.1.2 is available at the\n>> usual places:\n>\n> Well, by \"available\" you probably mean \"not available\", because it doesn't \n> actually work.\n>\n> I get EPERM on pack-e00affefe0f779d0f9b0507aef25a1733f4a9117.idx/pack, \n> because they are\n>\n> \t-r-------- 1 junio junio  1120880\n> \t-r-------- 1 junio junio 15709370\n>\n> respectively.\n\nI've run fsck and then flipped the bits manually on these files\nto unblock people, but I have no idea how this happened.  These\ntwo were the only files that had the funny bits in the\nrepository.\n\n        -r-------- 1 junio junio  1120880 Apr 22 06:46\n        -r-------- 1 junio junio 15709370 Apr 22 06:46\n\nI can tell from the output of \"last\" on that machine that I was\npresent and I am reasonably sure this was from \"git repack -a -d\"\nI manually did.  But my umask is 0002...\n\nGaah.  \"git repack -a -d\" from 'master' leaves packs with 0400\nmode (maint is fine).\n\n> Oops.\n\nOops indeed.\n"},{"id":"40135","messageId":"7vmz10nv1h.fsf_-_@assigned-by-dhcp.cox.net","threadId":"7621","inReplyTo":"7vvefonvdz.fsf@assigned-by-dhcp.cox.net","subject":"Today's 'master' leaves .idx/.pack in 0400","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-22T18:06:18Z","receivedAt":"2007-04-22T18:06:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With recent glibc, mkstemp() creates 0400 file.  Updated\npack-objects uses it in pack/idx writing without fixing this,\nhence this problem.\n\nWill have a fix hopefully shortly.\n"},{"id":"40137","messageId":"alpine.LFD.0.98.0704221422250.28339@xanadu.home","threadId":"7621","inReplyTo":"7vmz10nv1h.fsf_-_@assigned-by-dhcp.cox.net","subject":"Re: Today's 'master' leaves .idx/.pack in 0400","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-22T18:25:14Z","receivedAt":"2007-04-22T18:25:14Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sun, 22 Apr 2007, Junio C Hamano wrote:\n\n> With recent glibc, mkstemp() creates 0400 file.  Updated\n> pack-objects uses it in pack/idx writing without fixing this,\n> hence this problem.\n\nOops.  I guess I'm guilty for this.  I didn't bother looking at the \npermission on the pack for git-pack-objects since git-repack seemed to \ntake care of that.  But it only _remove_ write permissions.\n\n\nNicolas\n"},{"id":"40139","messageId":"7vejmcnu28.fsf@assigned-by-dhcp.cox.net","threadId":"7621","inReplyTo":"alpine.LFD.0.98.0704221422250.28339@xanadu.home","subject":"Re: Today's 'master' leaves .idx/.pack in 0400","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-22T18:27:27Z","receivedAt":"2007-04-22T18:27:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre <nico@cam.org> writes:\n\n> On Sun, 22 Apr 2007, Junio C Hamano wrote:\n>\n>> With recent glibc, mkstemp() creates 0400 file.  Updated\n>> pack-objects uses it in pack/idx writing without fixing this,\n>> hence this problem.\n>\n> Oops.  I guess I'm guilty for this.  I didn't bother looking at the \n> permission on the pack for git-pack-objects since git-repack seemed to \n> take care of that.  But it only _remove_ write permissions.\n\nOk, then probably we can change the 0444 in my \"quickfix\" patch\nto 0644.  That should also let the 5300 test pass.\n"},{"id":"40140","messageId":"alpine.LFD.0.98.0704221431430.28339@xanadu.home","threadId":"7621","inReplyTo":"7vejmcnu28.fsf@assigned-by-dhcp.cox.net","subject":"Re: Today's 'master' leaves .idx/.pack in 0400","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-22T18:34:09Z","receivedAt":"2007-04-22T18:34:09Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sun, 22 Apr 2007, Junio C Hamano wrote:\n\n> Nicolas Pitre <nico@cam.org> writes:\n> \n> > On Sun, 22 Apr 2007, Junio C Hamano wrote:\n> >\n> >> With recent glibc, mkstemp() creates 0400 file.  Updated\n> >> pack-objects uses it in pack/idx writing without fixing this,\n> >> hence this problem.\n> >\n> > Oops.  I guess I'm guilty for this.  I didn't bother looking at the \n> > permission on the pack for git-pack-objects since git-repack seemed to \n> > take care of that.  But it only _remove_ write permissions.\n> \n> Ok, then probably we can change the 0444 in my \"quickfix\" patch\n> to 0644.  That should also let the 5300 test pass.\n\nWell, actually there is no point making pack files writable.  If they're \nmodified, they get corrupted.\n\nHere's the fix I wanted to propose:\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex c72e07a..85c6e6e 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1786,11 +1786,13 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\tif (rename(pack_tmp_name, tmpname))\n \t\t\tdie(\"unable to rename temporary pack file: %s\",\n \t\t\t    strerror(errno));\n+\t\tchmod(tmpname, 0444);\n \t\tsnprintf(tmpname, sizeof(tmpname), \"%s-%s.idx\",\n \t\t\t base_name, sha1_to_hex(object_list_sha1));\n \t\tif (rename(idx_tmp_name, tmpname))\n \t\t\tdie(\"unable to rename temporary index file: %s\",\n \t\t\t    strerror(errno));\n+\t\tchmod(tmpname, 0444);\n \t\tputs(sha1_to_hex(object_list_sha1));\n \t}\n \tif (progress)\n\n\nNicolas\n"},{"id":"40141","messageId":"alpine.LFD.0.98.0704221443070.28339@xanadu.home","threadId":"7621","inReplyTo":"alpine.LFD.0.98.0704221431430.28339@xanadu.home","subject":"Re: Today's 'master' leaves .idx/.pack in 0400","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-22T18:47:13Z","receivedAt":"2007-04-22T18:47:13Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sun, 22 Apr 2007, Nicolas Pitre wrote:\n\n> On Sun, 22 Apr 2007, Junio C Hamano wrote:\n> \n> > Nicolas Pitre <nico@cam.org> writes:\n> > \n> > > On Sun, 22 Apr 2007, Junio C Hamano wrote:\n> > >\n> > >> With recent glibc, mkstemp() creates 0400 file.  Updated\n> > >> pack-objects uses it in pack/idx writing without fixing this,\n> > >> hence this problem.\n> > >\n> > > Oops.  I guess I'm guilty for this.  I didn't bother looking at the \n> > > permission on the pack for git-pack-objects since git-repack seemed to \n> > > take care of that.  But it only _remove_ write permissions.\n> > \n> > Ok, then probably we can change the 0444 in my \"quickfix\" patch\n> > to 0644.  That should also let the 5300 test pass.\n> \n> Well, actually there is no point making pack files writable.  If they're \n> modified, they get corrupted.\n\nOK there are those test cases.\n\nWell...  Either we chmod to 0644, or we fix the tests to 'chmod +w' like \nit is already done in t5302.\n\nIn any case I wouldn't die() but only error() on a failure to chmod().  \nIt is sure inconvenient if the pack isn't world readable, but it is not \na \"fatal\" problem for the repack.\n\n\nNicolas\n"},{"id":"40142","messageId":"7v7is4nt47.fsf@assigned-by-dhcp.cox.net","threadId":"7621","inReplyTo":"alpine.LFD.0.98.0704221019100.9964@woody.linux-foundation.org","subject":"Re: [ANNOUNCE] GIT 1.5.1.2","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-22T18:47:52Z","receivedAt":"2007-04-22T18:47:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Sat, 21 Apr 2007, Junio C Hamano wrote:\n>>\n>> The latest maintenance release GIT 1.5.1.2 is available at the\n>> usual places:\n>\n> Well, by \"available\" you probably mean \"not available\", because it doesn't \n> actually work.\n>\n> I get EPERM on pack-e00affefe0f779d0f9b0507aef25a1733f4a9117.idx/pack, \n> because they are\n>\n> \t-r-------- 1 junio junio  1120880\n> \t-r-------- 1 junio junio 15709370\n>\n> respectively.\n> ...\n> Oops.\n\nOk, to clarify the situation.\n\n * This breakage does not make v1.5.1.2 is a dud release, as what\n   caused this is not contained in it.\n\n * A topic to update git-pack-objects has been cooking in 'next'\n   and it graduated to 'master' last night after v1.5.1.2 was\n   cut from 'maint'.  This series had a bug that left permission\n   bits as set by mkstemp(); newer glibc leaves it as 0600.\n\n * I almost always run 'master' on kernel.org; I ran \"git-repack\n   -a -d\" there using 'master' that contained the breakage.\n\n * git-repack lets git-pack-objects to create pack/idx, and then\n   drops the write permission bits from them.  That is how they\n   got 0400.\n\n * I've fixed the pack/idx 0444 manually at kernel.org; the\n   repository has already mirrored out to git.kernel.org, so\n   people should be able to fetch from there now.  alt-git.git\n   at repo.or.cz should be usable as well, although I suspect\n   the site did not even have this issue.\n\n * I sent a quickfix as a workaround.  Because we rely on\n   git-repack to drop write permissions anyway, and some tests\n   to plumbing expects the resulting files to be writable by the\n   owner, tweaking 0444 in the quickfix to 0644 would be a good\n   workaround to fix the problem -and- pass all the test.\n\nSo I'll push out a 0644 quickfix on 'master' shortly.\n\nHowever.\n\n * To reiterate, people planning to update to v1.5.1.2 should\n   not be alarmed with this gotcha.  That comes from the\n   maintenance branch and is not affected by this breakage.\n\n * Letting mkstemp() to create the file and then forcing 0644 is\n   not strictly correct, as it makes the files created by people\n   with 007 umask readable to the general public.  The code\n   before mkstemp() conversion was aware of this issue and used\n   open(..., 0666) to let the user's umask applied, I think.\n\nSo while the quickfix is an improvement, it not a real fix X-<.\n"},{"id":"40143","messageId":"7v3b2snsx6.fsf@assigned-by-dhcp.cox.net","threadId":"7621","inReplyTo":"alpine.LFD.0.98.0704221443070.28339@xanadu.home","subject":"Re: Today's 'master' leaves .idx/.pack in 0400","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-22T18:52:05Z","receivedAt":"2007-04-22T18:52:05Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre <nico@cam.org> writes:\n\n> On Sun, 22 Apr 2007, Nicolas Pitre wrote:\n>\n>> On Sun, 22 Apr 2007, Junio C Hamano wrote:\n>> \n>> > Nicolas Pitre <nico@cam.org> writes:\n>> > \n>> > > On Sun, 22 Apr 2007, Junio C Hamano wrote:\n>> > >\n>> > >> With recent glibc, mkstemp() creates 0400 file.  Updated\n>> > >> pack-objects uses it in pack/idx writing without fixing this,\n>> > >> hence this problem.\n>> > >\n>> > > Oops.  I guess I'm guilty for this.  I didn't bother looking at the \n>> > > permission on the pack for git-pack-objects since git-repack seemed to \n>> > > take care of that.  But it only _remove_ write permissions.\n>> > \n>> > Ok, then probably we can change the 0444 in my \"quickfix\" patch\n>> > to 0644.  That should also let the 5300 test pass.\n>> \n>> Well, actually there is no point making pack files writable.  If they're \n>> modified, they get corrupted.\n>\n> OK there are those test cases.\n>\n> Well...  Either we chmod to 0644, or we fix the tests to 'chmod +w' like \n> it is already done in t5302.\n>\n> In any case I wouldn't die() but only error() on a failure to chmod().  \n> It is sure inconvenient if the pack isn't world readable, but it is not \n> a \"fatal\" problem for the repack.\n\nIf it DIED, I would not have to have been embarrassed in public\nlike this!\n\nGrumble.\n"},{"id":"40144","messageId":"alpine.LFD.0.98.0704221144250.9964@woody.linux-foundation.org","threadId":"7621","inReplyTo":"alpine.LFD.0.98.0704221431430.28339@xanadu.home","subject":"Re: Today's 'master' leaves .idx/.pack in 0400","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-22T18:54:41Z","receivedAt":"2007-04-22T18:54:41Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 22 Apr 2007, Nicolas Pitre wrote:\n> \n> Well, actually there is no point making pack files writable.  If they're \n> modified, they get corrupted.\n> \n> Here's the fix I wanted to propose:\n> \n> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> index c72e07a..85c6e6e 100644\n> --- a/builtin-pack-objects.c\n> +++ b/builtin-pack-objects.c\n> @@ -1786,11 +1786,13 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n>  \t\tif (rename(pack_tmp_name, tmpname))\n>  \t\t\tdie(\"unable to rename temporary pack file: %s\",\n>  \t\t\t    strerror(errno));\n> +\t\tchmod(tmpname, 0444);\n\nI think this is wrong (as is Junio's). I think we should still honor the \nrepository permission setting, and default to honoring umask.\n\nSo I think that if the user has a umask that says \"nobody else can read\", \nthen we should *not* make it world readable (unless the \n\"shared_repository\" thing is set to override it, of course).\n\n\t\tLinus\n"},{"id":"40145","messageId":"7vy7kkmdtg.fsf@assigned-by-dhcp.cox.net","threadId":"7621","inReplyTo":"alpine.LFD.0.98.0704221144250.9964@woody.linux-foundation.org","subject":"Re: Today's 'master' leaves .idx/.pack in 0400","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-22T19:03:39Z","receivedAt":"2007-04-22T19:03:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Sun, 22 Apr 2007, Nicolas Pitre wrote:\n>> \n>> Well, actually there is no point making pack files writable.  If they're \n>> modified, they get corrupted.\n>> \n>> Here's the fix I wanted to propose:\n>> \n>> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n>> index c72e07a..85c6e6e 100644\n>> --- a/builtin-pack-objects.c\n>> +++ b/builtin-pack-objects.c\n>> @@ -1786,11 +1786,13 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n>>  \t\tif (rename(pack_tmp_name, tmpname))\n>>  \t\t\tdie(\"unable to rename temporary pack file: %s\",\n>>  \t\t\t    strerror(errno));\n>> +\t\tchmod(tmpname, 0444);\n>\n> I think this is wrong (as is Junio's). I think we should still honor the \n> repository permission setting, and default to honoring umask.\n>\n> So I think that if the user has a umask that says \"nobody else can read\", \n> then we should *not* make it world readable (unless the \n> \"shared_repository\" thing is set to override it, of course).\n\nI obviously agree, but as a tentative measure, I'll push out\n0644 version anyway.\n"},{"id":"40146","messageId":"7vd51wmd1a.fsf@assigned-by-dhcp.cox.net","threadId":"7621","inReplyTo":"7vy7kkmdtg.fsf@assigned-by-dhcp.cox.net","subject":"Re: Today's 'master' leaves .idx/.pack in 0400","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-22T19:20:33Z","receivedAt":"2007-04-22T19:20:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> Linus Torvalds <torvalds@linux-foundation.org> writes:\n>\n>> So I think that if the user has a umask that says \"nobody else can read\", \n>> then we should *not* make it world readable (unless the \n>> \"shared_repository\" thing is set to override it, of course).\n>\n> I obviously agree, but as a tentative measure, I'll push out\n> 0644 version anyway.\n\nHow about this as a replacement (hot off the press -- still\nrunning the tests).\n\n-- >8 --\npack-objects: adjust the permission bits of created files.\n\nThe updated pack-objects let mkstemp() to create new pack/idx\npair, without fixing the permission bits on them.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n builtin-pack-objects.c |   18 ++++++++++++++++++\n 1 files changed, 18 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex c72e07a..34350bf 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1612,6 +1612,13 @@ static void get_object_list(int ac, const char **av)\n \ttraverse_commit_list(&revs, show_commit, show_object);\n }\n \n+static int adjust_perm(const char *path, mode_t mode)\n+{\n+\tif (chmod(path, mode))\n+\t\treturn -1;\n+\treturn adjust_shared_perm(path);\n+}\n+\n int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n {\n \tint depth = 10;\n@@ -1780,14 +1787,25 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tlast_obj_offset = write_pack_file();\n \tif (!pack_to_stdout) {\n \t\tunsigned char object_list_sha1[20];\n+\t\tmode_t mode = umask(0);\n+\n+\t\tumask(mode);\n+\t\tmode = 0666 & ~mode;\n+\n \t\twrite_index_file(last_obj_offset, object_list_sha1);\n \t\tsnprintf(tmpname, sizeof(tmpname), \"%s-%s.pack\",\n \t\t\t base_name, sha1_to_hex(object_list_sha1));\n+\t\tif (adjust_perm(pack_tmp_name, mode))\n+\t\t\tdie(\"unable to make temporary pack file readable: %s\",\n+\t\t\t    strerror(errno));\n \t\tif (rename(pack_tmp_name, tmpname))\n \t\t\tdie(\"unable to rename temporary pack file: %s\",\n \t\t\t    strerror(errno));\n \t\tsnprintf(tmpname, sizeof(tmpname), \"%s-%s.idx\",\n \t\t\t base_name, sha1_to_hex(object_list_sha1));\n+\t\tif (adjust_perm(idx_tmp_name, mode))\n+\t\t\tdie(\"unable to make temporary index file readable: %s\",\n+\t\t\t    strerror(errno));\n \t\tif (rename(idx_tmp_name, tmpname))\n \t\t\tdie(\"unable to rename temporary index file: %s\",\n \t\t\t    strerror(errno));\n"},{"id":"40147","messageId":"alpine.LFD.0.98.0704221225300.9964@woody.linux-foundation.org","threadId":"7621","inReplyTo":"7vd51wmd1a.fsf@assigned-by-dhcp.cox.net","subject":"Re: Today's 'master' leaves .idx/.pack in 0400","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-22T19:29:55Z","receivedAt":"2007-04-22T19:29:55Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 22 Apr 2007, Junio C Hamano wrote:\n> \n> How about this as a replacement (hot off the press -- still\n> running the tests).\n\nLooks good, but..\n\n> +\t\tmode_t mode = umask(0);\n> +\n> +\t\tumask(mode);\n> +\t\tmode = 0666 & ~mode;\n\nI would really suggest just defaulting to\n\n\tmode = 0444 & ~mode;\n\nsince there simply is never any reason to allow a writable pack-file.\n\nThe fact that we have some tests that try to corrupt a pack-file is not \nreally a reason. Just make them do \"chmod +w\" before corrupting it.\n\nBut your patch is an obvious improvement regardless, so I certainly don't \nthink this is a *big* issue.\n\n\t\tLinus\n"}]}