{"thread":{"id":"11654","subject":"What's in git.git (stable frozen)","startedAt":"2008-01-17T03:01:00Z","lastAt":"2008-01-18T03:01:46Z","messageCount":8,"participants":["Junio C Hamano","Johannes Sixt","Brandon Casey","Shawn O. Pearce"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"65677","messageId":"7vk5m9kvf7.fsf@gitster.siamese.dyndns.org","threadId":"11654","inReplyTo":null,"subject":"What's in git.git (stable frozen)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-17T03:01:00Z","receivedAt":"2008-01-17T03:01:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"There are a good deal of bugfixes in 'master', the largest of\nwhich is Brandon Casey's fix to builtin-commit and others'\nmisuse of lockfile API.  A tentative fix for the issue was\npushed out last night but the approach has known issues for our\nWindowsy friends, and this attempts to address them.\n\nWe've been taking pride that the tip of 'master' is always,\nwithout regression, more stable than any released version, but\ntoday's one might have uncovered glitches.  Please help testing\nit so that we do not have to leave it broken for a long time if\nit indeed is.\n\nI spent quite a lot of time trying to come up with a simple\nreproduction recipe for the builtin-commit breakage, but I\ncouldn't.  I also wanted to apply the patch to remove the use\n(but not implementation) of repo-config along with a removal\nnotice in the release notes but haven't got around to.\n\nAside from obviously correct trivial fixes that may come from\nthe list, what currently is not in 'master' but should be in the\nrc4 are very minimal.  Brandon Casey's lockfile fix needs to be\ntested for some time, repo-config deprecation notice needs to be\nposted, and \"\\C-+\" patch for gitk needs to be pulled from Paul.\nI see French translation for git-gui in Shawn's repository, but\nI do not know if it, along with other changes to git-gui, are\nmeant for 1.5.4 or not.  If they are, I'd like to include them\nall before tagging rc4.\n\n----------------------------------------------------------------\n\n* The 'master' branch has these since the last announcement\n  in addition to the above.\n\nBill Lear (1):\n  Correct spelling in diff.c comment\n\nBrandon Casey (3):\n  close_lock_file(): new function in the lockfile API\n  Improve use of lockfile API\n  refs.c: rework ref_locks by abstracting from underlying struct lock_file\n\nDan McGee (2):\n  Remove usage of git- (dash) commands from email hook\n  cvsimport: remove last use of repo-config from git standard tools\n\nDave Peticolas (1):\n  Documentation: fix and clarify grammar in git-merge docs.\n\nDmitry Potapov (1):\n  treat any file with NUL as binary\n\nJean-Luc Herren (1):\n  Make default pre-commit hook less noisy\n\nJunio C Hamano (4):\n  Revert \"builtin-commit.c: remove useless check added by faulty cut and\n    paste\"\n  Fix git-rerere documentation\n  Squelch bogus progress output from git-rebase--interactive\n  Document lockfile API\n\nKristian Høgsberg (1):\n  git-commit: fix double close(2) that can close a wrong file descriptor\n\nLinus Torvalds (3):\n  Make builtin-commit.c more careful about parenthood\n  Make 'git fsck' complain about non-commit branches\n  Be more careful about updating refs\n\nMark Drago (1):\n  hg-to-git: improve popen calls\n\nMiklos Vajna (2):\n  Add using merge subtree How-To\n  ls-remote: add -t and -h options.\n"},{"id":"65712","messageId":"478F5070.2020503@viscovery.net","threadId":"11654","inReplyTo":"7vk5m9kvf7.fsf@gitster.siamese.dyndns.org","subject":"Re: What's in git.git (stable frozen)","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-01-17T12:56:16Z","receivedAt":"2008-01-17T12:56:16Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Junio C Hamano schrieb:\n> There are a good deal of bugfixes in 'master', the largest of\n> which is Brandon Casey's fix to builtin-commit and others'\n> misuse of lockfile API.  A tentative fix for the issue was\n> pushed out last night but the approach has known issues for our\n> Windowsy friends, and this attempts to address them.\n> \n> We've been taking pride that the tip of 'master' is always,\n> without regression, more stable than any released version, but\n> today's one might have uncovered glitches.  Please help testing\n> it so that we do not have to leave it broken for a long time if\n> it indeed is.\n\nExcept for the NO_MMAP issue introduced by c3b0dec509f that kills\ngit-fast-import in t9301* as discussed in a parallel thread, all tests\npass on Windows (MinGW port). This is a good sign since most, if not all,\nissues around the lockfile API have been discovered by the test suite in\nthe past.\n\n-- Hannes\n"},{"id":"65733","messageId":"478F893A.6050105@nrlssc.navy.mil","threadId":"11654","inReplyTo":"7vk5m9kvf7.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] fast-import.c: don't try to commit marks file if write failed","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-01-17T16:58:34Z","receivedAt":"2008-01-17T16:58:34Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"We also move the assignment of -1 to the lock file descriptor\nup, so that rollback_lock_file() can be called safely after a\npossible attempt to fclose(). This matches the contents of\nthe 'if' statement just above testing success of fdopen().\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n\n\nSince we forget the lock file descriptor there is a chance\nthat we will leave the file open if a write error occurs. We'll\nstill delete the file. I don't think it's worth bending\nover backwards to make sure the file is closed on this failure.\n\n-brandon\n\n\n fast-import.c |   24 ++++++++++++++++--------\n 1 files changed, 16 insertions(+), 8 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 3609c24..4cf5092 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1545,19 +1545,27 @@ static void dump_marks(void)\n \t\treturn;\n \t}\n \n-\tdump_marks_helper(f, 0, marks);\n-\tif (ferror(f) || fclose(f))\n-\t\tfailure |= error(\"Unable to write marks file %s: %s\",\n-\t\t\tmark_file, strerror(errno));\n \t/*\n-\t * Since the lock file was fdopen()'ed and then fclose()'ed above,\n-\t * assign -1 to the lock file descriptor so that commit_lock_file()\n+\t * Since the lock file was fdopen()'ed, it should not be close()'ed.\n+\t * Assign -1 to the lock file descriptor so that commit_lock_file()\n \t * won't try to close() it.\n \t */\n \tmark_lock.fd = -1;\n-\tif (commit_lock_file(&mark_lock))\n-\t\tfailure |= error(\"Unable to write commit file %s: %s\",\n+\n+\tdump_marks_helper(f, 0, marks);\n+\tif (ferror(f) || fclose(f)) {\n+\t\trollback_lock_file(&mark_lock);\n+\t\tfailure |= error(\"Unable to write marks file %s: %s\",\n \t\t\tmark_file, strerror(errno));\n+\t\treturn;\n+\t}\n+\n+\tif (commit_lock_file(&mark_lock)) {\n+\t\trollback_lock_file(&mark_lock);\n+\t\tfailure |= error(\"Unable to commit marks file %s: %s\",\n+\t\t\tmark_file, strerror(errno));\n+\t\treturn;\n+\t}\n }\n \n static int read_next_command(void)\n-- \n1.5.4.rc3.17.gb63a4\n"},{"id":"65734","messageId":"478F8D9B.5080703@nrlssc.navy.mil","threadId":"11654","inReplyTo":"7vk5m9kvf7.fsf@gitster.siamese.dyndns.org","subject":"Re: What's in git.git (stable frozen)","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-01-17T17:17:15Z","receivedAt":"2008-01-17T17:17:15Z","isPatch":false,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"\n\nIn lockfile.c:commit_locked_index()\n\nYou didn't include the portion of your patch that\nassigned NULL to alternate_index_output before\nattempting to close and rename.\n\nShould that be included?\n\n-brandon\n\n\n\nJunio C Hamano wrote:\n\n@@ -185,9 +198,15 @@ void set_alternate_index_output(const char *name)\n int commit_locked_index(struct lock_file *lk)\n {\n \tif (alternate_index_output) {\n-\t\tint result = rename(lk->filename, alternate_index_output);\n-\t\tlk->filename[0] = 0;\n-\t\treturn result;\n+\t\tconst char *newname = alternate_index_output;\n+\t\talternate_index_output = NULL;\n"},{"id":"65742","messageId":"7vtzlcjpx8.fsf@gitster.siamese.dyndns.org","threadId":"11654","inReplyTo":"478F8D9B.5080703@nrlssc.navy.mil","subject":"Re: What's in git.git (stable frozen)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-17T17:57:23Z","receivedAt":"2008-01-17T17:57:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey <casey@nrlssc.navy.mil> writes:\n\n> In lockfile.c:commit_locked_index()\n>\n> You didn't include the portion of your patch that\n> assigned NULL to alternate_index_output before\n> attempting to close and rename.\n>\n> Should that be included?\n\nYou are right, but that is a separate topic and should be a\nseparate patch, so I dropped the hunk from that commit.\n"},{"id":"65741","messageId":"7vmyr4jpww.fsf@gitster.siamese.dyndns.org","threadId":"11654","inReplyTo":"478F893A.6050105@nrlssc.navy.mil","subject":"Re: [PATCH] fast-import.c: don't try to commit marks file if write failed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-17T17:57:35Z","receivedAt":"2008-01-17T17:57:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey <casey@nrlssc.navy.mil> writes:\n\n> We also move the assignment of -1 to the lock file descriptor\n> up, so that rollback_lock_file() can be called safely after a\n> possible attempt to fclose(). This matches the contents of\n> the 'if' statement just above testing success of fdopen().\n>\n> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n> ---\n\nShawn, could you lend an extra set of eyeballs on this one\nplease?  It looks sane to me but obviously you are more familiar\nwith the code.\n\n> Since we forget the lock file descriptor there is a chance\n> that we will leave the file open if a write error occurs. We'll\n> still delete the file. I don't think it's worth bending\n> over backwards to make sure the file is closed on this failure.\n>\n> -brandon\n>\n>\n>  fast-import.c |   24 ++++++++++++++++--------\n>  1 files changed, 16 insertions(+), 8 deletions(-)\n>\n> diff --git a/fast-import.c b/fast-import.c\n> index 3609c24..4cf5092 100644\n> --- a/fast-import.c\n> +++ b/fast-import.c\n> @@ -1545,19 +1545,27 @@ static void dump_marks(void)\n>  \t\treturn;\n>  \t}\n>  \n> -\tdump_marks_helper(f, 0, marks);\n> -\tif (ferror(f) || fclose(f))\n> -\t\tfailure |= error(\"Unable to write marks file %s: %s\",\n> -\t\t\tmark_file, strerror(errno));\n>  \t/*\n> -\t * Since the lock file was fdopen()'ed and then fclose()'ed above,\n> -\t * assign -1 to the lock file descriptor so that commit_lock_file()\n> +\t * Since the lock file was fdopen()'ed, it should not be close()'ed.\n> +\t * Assign -1 to the lock file descriptor so that commit_lock_file()\n>  \t * won't try to close() it.\n>  \t */\n>  \tmark_lock.fd = -1;\n> -\tif (commit_lock_file(&mark_lock))\n> -\t\tfailure |= error(\"Unable to write commit file %s: %s\",\n> +\n> +\tdump_marks_helper(f, 0, marks);\n> +\tif (ferror(f) || fclose(f)) {\n> +\t\trollback_lock_file(&mark_lock);\n> +\t\tfailure |= error(\"Unable to write marks file %s: %s\",\n>  \t\t\tmark_file, strerror(errno));\n> +\t\treturn;\n> +\t}\n> +\n> +\tif (commit_lock_file(&mark_lock)) {\n> +\t\trollback_lock_file(&mark_lock);\n> +\t\tfailure |= error(\"Unable to commit marks file %s: %s\",\n> +\t\t\tmark_file, strerror(errno));\n> +\t\treturn;\n> +\t}\n>  }\n>  \n>  static int read_next_command(void)\n> -- \n> 1.5.4.rc3.17.gb63a4\n"},{"id":"65807","messageId":"20080118020911.GQ24004@spearce.org","threadId":"11654","inReplyTo":"7vmyr4jpww.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] fast-import.c: don't try to commit marks file if write failed","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-01-18T02:09:12Z","receivedAt":"2008-01-18T02:09:12Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> Brandon Casey <casey@nrlssc.navy.mil> writes:\n> \n> > We also move the assignment of -1 to the lock file descriptor\n> > up, so that rollback_lock_file() can be called safely after a\n> > possible attempt to fclose(). This matches the contents of\n> > the 'if' statement just above testing success of fdopen().\n> >\n> > Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n> > ---\n> \n> Shawn, could you lend an extra set of eyeballs on this one\n> please?  It looks sane to me but obviously you are more familiar\n> with the code.\n\nI agree.\n\nAcked-by: Shawn O. Pearce <spearce@spearce.org>\n \n-- \nShawn.\n"},{"id":"65813","messageId":"20080118030146.GV24004@spearce.org","threadId":"11654","inReplyTo":"7vk5m9kvf7.fsf@gitster.siamese.dyndns.org","subject":"Re: What's in git.git (stable frozen)","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-01-18T03:01:46Z","receivedAt":"2008-01-18T03:01:46Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> I see French translation for git-gui in Shawn's repository, but\n> I do not know if it, along with other changes to git-gui, are\n> meant for 1.5.4 or not.  If they are, I'd like to include them\n> all before tagging rc4.\n\nI have a few changes queued for 0.9.2, which I'd like to get done\nand included before 1.5.4, plus I'd like to get Mark Levedahl's\nDESTDIR issue resolved before then too.\n\n\n.... changes since 0.9.1 ....\n\nChristian Couder (2):\n      git-gui: Initial french translation\n      git-gui: add french glossary: glossary/fr.po\n\nChristian Stimming (4):\n      git-gui: Update glossary: add term \"hunk\"\n      git-gui: Update German translation\n      git-gui: Fix broken revert confirmation.\n      git-gui: Improve German translation.\n\nPeter Karlsson (1):\n      git-gui: Updated Swedish translation after mailing list review.\n\nShawn O. Pearce (2):\n      git-gui: Allow 'Create New Repository' on existing directories\n      git-gui: Refresh file status description after hunk application\n\n-- \nShawn.\n"}]}