What's in git.git (stable frozen)

What's in git.git (stable frozen)

[Junio C Hamano]

There are a good deal of bugfixes in 'master', the largest of
which is Brandon Casey's fix to builtin-commit and others'
misuse of lockfile API.  A tentative fix for the issue was
pushed out last night but the approach has known issues for our
Windowsy friends, and this attempts to address them.

We've been taking pride that the tip of 'master' is always,
without regression, more stable than any released version, but
today's one might have uncovered glitches.  Please help testing
it so that we do not have to leave it broken for a long time if
it indeed is.

I spent quite a lot of time trying to come up with a simple
reproduction recipe for the builtin-commit breakage, but I
couldn't.  I also wanted to apply the patch to remove the use
(but not implementation) of repo-config along with a removal
notice in the release notes but haven't got around to.

Aside from obviously correct trivial fixes that may come from
the list, what currently is not in 'master' but should be in the
rc4 are very minimal.  Brandon Casey's lockfile fix needs to be
tested for some time, repo-config deprecation notice needs to be
posted, and "\C-+" patch for gitk needs to be pulled from Paul.
I see French translation for git-gui in Shawn's repository, but
I do not know if it, along with other changes to git-gui, are
meant for 1.5.4 or not.  If they are, I'd like to include them
all before tagging rc4.

----------------------------------------------------------------

* The 'master' branch has these since the last announcement
  in addition to the above.

Bill Lear (1):
  Correct spelling in diff.c comment

Brandon Casey (3):
  close_lock_file(): new function in the lockfile API
  Improve use of lockfile API
  refs.c: rework ref_locks by abstracting from underlying struct lock_file

Dan McGee (2):
  Remove usage of git- (dash) commands from email hook
  cvsimport: remove last use of repo-config from git standard tools

Dave Peticolas (1):
  Documentation: fix and clarify grammar in git-merge docs.

Dmitry Potapov (1):
  treat any file with NUL as binary

Jean-Luc Herren (1):
  Make default pre-commit hook less noisy

Junio C Hamano (4):
  Revert "builtin-commit.c: remove useless check added by faulty cut and
    paste"
  Fix git-rerere documentation
  Squelch bogus progress output from git-rebase--interactive
  Document lockfile API

Kristian Høgsberg (1):
  git-commit: fix double close(2) that can close a wrong file descriptor

Linus Torvalds (3):
  Make builtin-commit.c more careful about parenthood
  Make 'git fsck' complain about non-commit branches
  Be more careful about updating refs

Mark Drago (1):
  hg-to-git: improve popen calls

Miklos Vajna (2):
  Add using merge subtree How-To
  ls-remote: add -t and -h options.

Re: What's in git.git (stable frozen)

[Johannes Sixt]

Junio C Hamano schrieb:
> There are a good deal of bugfixes in 'master', the largest of
> which is Brandon Casey's fix to builtin-commit and others'
> misuse of lockfile API.  A tentative fix for the issue was
> pushed out last night but the approach has known issues for our
> Windowsy friends, and this attempts to address them.
> 
> We've been taking pride that the tip of 'master' is always,
> without regression, more stable than any released version, but
> today's one might have uncovered glitches.  Please help testing
> it so that we do not have to leave it broken for a long time if
> it indeed is.

Except for the NO_MMAP issue introduced by c3b0dec509f that kills
git-fast-import in t9301* as discussed in a parallel thread, all tests
pass on Windows (MinGW port). This is a good sign since most, if not all,
issues around the lockfile API have been discovered by the test suite in
the past.

-- Hannes

[PATCH] fast-import.c: don't try to commit marks file if write failed

[Brandon Casey]

We also move the assignment of -1 to the lock file descriptor
up, so that rollback_lock_file() can be called safely after a
possible attempt to fclose(). This matches the contents of
the 'if' statement just above testing success of fdopen().

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---


Since we forget the lock file descriptor there is a chance
that we will leave the file open if a write error occurs. We'll
still delete the file. I don't think it's worth bending
over backwards to make sure the file is closed on this failure.

-brandon


 fast-import.c |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 3609c24..4cf5092 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1545,19 +1545,27 @@ static void dump_marks(void)
 		return;
 	}
 
-	dump_marks_helper(f, 0, marks);
-	if (ferror(f) || fclose(f))
-		failure |= error("Unable to write marks file %s: %s",
-			mark_file, strerror(errno));
 	/*
-	 * Since the lock file was fdopen()'ed and then fclose()'ed above,
-	 * assign -1 to the lock file descriptor so that commit_lock_file()
+	 * Since the lock file was fdopen()'ed, it should not be close()'ed.
+	 * Assign -1 to the lock file descriptor so that commit_lock_file()
 	 * won't try to close() it.
 	 */
 	mark_lock.fd = -1;
-	if (commit_lock_file(&mark_lock))
-		failure |= error("Unable to write commit file %s: %s",
+
+	dump_marks_helper(f, 0, marks);
+	if (ferror(f) || fclose(f)) {
+		rollback_lock_file(&mark_lock);
+		failure |= error("Unable to write marks file %s: %s",
 			mark_file, strerror(errno));
+		return;
+	}
+
+	if (commit_lock_file(&mark_lock)) {
+		rollback_lock_file(&mark_lock);
+		failure |= error("Unable to commit marks file %s: %s",
+			mark_file, strerror(errno));
+		return;
+	}
 }
 
 static int read_next_command(void)
-- 
1.5.4.rc3.17.gb63a4

Re: [PATCH] fast-import.c: don't try to commit marks file if write failed

[Junio C Hamano]

Brandon Casey <casey@nrlssc.navy.mil> writes:

> We also move the assignment of -1 to the lock file descriptor
> up, so that rollback_lock_file() can be called safely after a
> possible attempt to fclose(). This matches the contents of
> the 'if' statement just above testing success of fdopen().
>
> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> ---

Shawn, could you lend an extra set of eyeballs on this one
please?  It looks sane to me but obviously you are more familiar
with the code.

> Since we forget the lock file descriptor there is a chance
> that we will leave the file open if a write error occurs. We'll
> still delete the file. I don't think it's worth bending
> over backwards to make sure the file is closed on this failure.
>
> -brandon
>
>
>  fast-import.c |   24 ++++++++++++++++--------
>  1 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/fast-import.c b/fast-import.c
> index 3609c24..4cf5092 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -1545,19 +1545,27 @@ static void dump_marks(void)
>  		return;
>  	}
>  
> -	dump_marks_helper(f, 0, marks);
> -	if (ferror(f) || fclose(f))
> -		failure |= error("Unable to write marks file %s: %s",
> -			mark_file, strerror(errno));
>  	/*
> -	 * Since the lock file was fdopen()'ed and then fclose()'ed above,
> -	 * assign -1 to the lock file descriptor so that commit_lock_file()
> +	 * Since the lock file was fdopen()'ed, it should not be close()'ed.
> +	 * Assign -1 to the lock file descriptor so that commit_lock_file()
>  	 * won't try to close() it.
>  	 */
>  	mark_lock.fd = -1;
> -	if (commit_lock_file(&mark_lock))
> -		failure |= error("Unable to write commit file %s: %s",
> +
> +	dump_marks_helper(f, 0, marks);
> +	if (ferror(f) || fclose(f)) {
> +		rollback_lock_file(&mark_lock);
> +		failure |= error("Unable to write marks file %s: %s",
>  			mark_file, strerror(errno));
> +		return;
> +	}
> +
> +	if (commit_lock_file(&mark_lock)) {
> +		rollback_lock_file(&mark_lock);
> +		failure |= error("Unable to commit marks file %s: %s",
> +			mark_file, strerror(errno));
> +		return;
> +	}
>  }
>  
>  static int read_next_command(void)
> -- 
> 1.5.4.rc3.17.gb63a4

Re: [PATCH] fast-import.c: don't try to commit marks file if write failed

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> Brandon Casey <casey@nrlssc.navy.mil> writes:
> 
> > We also move the assignment of -1 to the lock file descriptor
> > up, so that rollback_lock_file() can be called safely after a
> > possible attempt to fclose(). This matches the contents of
> > the 'if' statement just above testing success of fdopen().
> >
> > Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> > ---
> 
> Shawn, could you lend an extra set of eyeballs on this one
> please?  It looks sane to me but obviously you are more familiar
> with the code.

I agree.

Acked-by: Shawn O. Pearce <spearce@spearce.org>
 
-- 
Shawn.

Re: What's in git.git (stable frozen)

[Brandon Casey]

In lockfile.c:commit_locked_index()

You didn't include the portion of your patch that
assigned NULL to alternate_index_output before
attempting to close and rename.

Should that be included?

-brandon



Junio C Hamano wrote:

@@ -185,9 +198,15 @@ void set_alternate_index_output(const char *name)
 int commit_locked_index(struct lock_file *lk)
 {
 	if (alternate_index_output) {
-		int result = rename(lk->filename, alternate_index_output);
-		lk->filename[0] = 0;
-		return result;
+		const char *newname = alternate_index_output;
+		alternate_index_output = NULL;

Re: What's in git.git (stable frozen)

[Junio C Hamano]

Brandon Casey <casey@nrlssc.navy.mil> writes:

> In lockfile.c:commit_locked_index()
>
> You didn't include the portion of your patch that
> assigned NULL to alternate_index_output before
> attempting to close and rename.
>
> Should that be included?

You are right, but that is a separate topic and should be a
separate patch, so I dropped the hunk from that commit.

Re: What's in git.git (stable frozen)

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> I see French translation for git-gui in Shawn's repository, but
> I do not know if it, along with other changes to git-gui, are
> meant for 1.5.4 or not.  If they are, I'd like to include them
> all before tagging rc4.

I have a few changes queued for 0.9.2, which I'd like to get done
and included before 1.5.4, plus I'd like to get Mark Levedahl's
DESTDIR issue resolved before then too.


.... changes since 0.9.1 ....

Christian Couder (2):
      git-gui: Initial french translation
      git-gui: add french glossary: glossary/fr.po

Christian Stimming (4):
      git-gui: Update glossary: add term "hunk"
      git-gui: Update German translation
      git-gui: Fix broken revert confirmation.
      git-gui: Improve German translation.

Peter Karlsson (1):
      git-gui: Updated Swedish translation after mailing list review.

Shawn O. Pearce (2):
      git-gui: Allow 'Create New Repository' on existing directories
      git-gui: Refresh file status description after hunk application

-- 
Shawn.