Git development
 help / color / mirror / Atom feed
* Re: [PATCH] reflog documentation: -n is an alias for --dry-run
From: Erik Faye-Lund @ 2009-09-13 13:05 UTC (permalink / raw)
  To: Jeff King; +Cc: Nelson Elhage, git
In-Reply-To: <20090913094032.GC14438@coredump.intra.peff.net>

On Sun, Sep 13, 2009 at 11:40 AM, Jeff King <peff@peff.net> wrote:
>>  static const char reflog_expire_usage[] =
>> -"git reflog (show|expire) [--verbose] [--dry-run] [--stale-fix] [--expire=<time>] [--expire-unreachable=<time>] [--all] <refs>...";
>> +"git reflog (show|expire) [--verbose] [-n | --dry-run] [--stale-fix] [--expire=<time>] [--expire-unreachable=<time>] [--all] <refs>...";
>
> Really? I think "git reflog show -n" is not about dry-run at all...

Indeed. However, the reflog expire uses -n as an alias for --dry-run,
according to lines 548-549 of my builtin-reflog.c (ddfdf5a):

		if (!strcmp(arg, "--dry-run") || !strcmp(arg, "-n"))
			cb.dry_run = 1;

...and this usage-string is only output from cmd_reflog_expire, so the
correct solution would IMO be to simply remove show from the
usage-string:

-"git reflog (show|expire) [--verbose] [-n | --dry-run] [--stale-fix]
[--expire=<time>] [--expire-unreachable=<time>] [--all] <refs>...";
+"git reflog expire [--verbose] [-n | --dry-run] [--stale-fix]
[--expire=<time>] [--expire-unreachable=<time>] [--all] <refs>...";

This issue ("reflog expire" incorrectly documenting "reflog show") was
present before this patch, though. At least the option "--stale-fix"
appears not to be valid for "reflog show".

-- 
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656

^ permalink raw reply

* Re: [PATCH] reflog documentation: -n is an alias for --dry-run
From: Nelson Elhage @ 2009-09-13 12:53 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20090913094032.GC14438@coredump.intra.peff.net>

Oh, I see. I was reading builtin-reflog.c, which, in both
cmd_reflog_expire and cmd_reflog_delete, does check for -n as
synonymous to --dry-run:

if (!strcmp(arg, "--dry-run") || !strcmp(arg, "-n"))
    cb.dry_run = 1;

but I missed that 'reflog show' aliases for git-log, and that that
accepts -n.

Patch withdrawn -- I'd send one documenting that -n works for delete
and expire, but it'd probably just complicate the documentation more
than clarify anything.

- Nelson

On Sun, Sep 13, 2009 at 05:40:32AM -0400, Jeff King wrote:
> On Sat, Sep 12, 2009 at 11:41:54PM -0400, Nelson Elhage wrote:
> 
> >  static const char reflog_expire_usage[] =
> > -"git reflog (show|expire) [--verbose] [--dry-run] [--stale-fix] [--expire=<time>] [--expire-unreachable=<time>] [--all] <refs>...";
> > +"git reflog (show|expire) [--verbose] [-n | --dry-run] [--stale-fix] [--expire=<time>] [--expire-unreachable=<time>] [--all] <refs>...";
> 
> Really? I think "git reflog show -n" is not about dry-run at all...
> 
> -Peff

^ permalink raw reply

* rename tracking and file-name swapping
From: Yuri D'Elia @ 2009-09-13 11:17 UTC (permalink / raw)
  To: git

Hi everyone. Does rename tracking recognize two file names being swapped?

% ls -l
total 24
-rw-rw-r--   1 wavexx  wavexx  5952 Sep 13 13:09 file1.txt
-rw-rw-r--   1 wavexx  wavexx  3330 Sep 13 13:09 file2.txt
% mv file1.txt file3.txt 
% mv file2.txt file1.txt
% mv file3.txt file2.txt
% git add file1.txt file2.txt 
% git diff -M --stat --cached
 file1.txt |  150 +++++++++++++++++++++++-------------------------------------
 file2.txt |  150 +++++++++++++++++++++++++++++++++++++-----------------------
 2 files changed, 150 insertions(+), 150 deletions(-)

I would expect at least to see

  file1.txt => file2.txt
  file2.txt => file1.txt

if the contents are totally unchanged.

Am I doing something wrong?

Thanks

^ permalink raw reply

* Re: git push --confirm ?
From: Jeff King @ 2009-09-13 10:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Owen Taylor, git, Colin Walters
In-Reply-To: <7vljkjuo43.fsf@alter.siamese.dyndns.org>

On Sun, Sep 13, 2009 at 03:37:32AM -0700, Junio C Hamano wrote:

> With --confirm, the wait happens while the --confirm waits for the human,
> and perhaps the command does "git log --oneline old...new" as convenience.
> While all this is happening, the TCP connection to the remote end is still
> kept open.  We do not lock anything, but if somebody else pushed from
> sideways, at the end of this session we would notice that, and the push
> will be aborted.
> 
> This somewhat makes me worry about DoS point of view, but it does make it
> somewhat safer.

I don't see how it makes a DoS any worse. A malicious attacker can
always open the TCP connection and let it sit; we are changing only the
client code, after all.

It does increase the possibility of _accidentally_ wasting a TCP
connection. I don't know if that is a real-world problem or not. I would
think heavily-utilized sites might put a time-out on the connection to
avoid such a DoS in the first place.

However, such a timeout is perhaps reason for us to be concerned with
implementing this feature with a single session. Will users looking at
the commits for confirmation delay enough to hit configured timeouts,
dropping their connection and forcing them to start again?

One other way to implement this would be with two TCP connections:

  1. git push --dry-run, recording <old-sha1> for each ref to be pushed.
     Afterwards, drop the TCP connection.

  2. Get confirmation from the user.

  3. Do the push again, confirming that the <old-sha1> values sent by
     the server match what we showed the user for confirmation. If not,
     abort the push.

Besides being a lot more annoying to implement, there is one big
downside: in many cases the single TCP connection is a _feature_. If you
are pushing via ssh and providing a password manually, it is a
significant usability regression to have to input it twice.

Also, given that ssh is going to be by far the biggest transport for
pushing via the git protocol, I suspect any timeouts are set for
_before_ the authentication phase (i.e., SSH times you out if you don't
actually log in). So in that sense it may not be worth worrying about
how long we take during the push itself.

> I think the largest practical safety would come from the fact that this
> would make it convenient (i.e. a single command "push --confirm") than
> having to run two separate ones with manual inspection in between.  A
> safety feature that is cumbersome to use won't add much to safety, as that
> is unlikely to be used in the first place.

Sure. But that is about packaging it up as a single session for the
user. If there is no concern about atomicity, you could do that with a
simple wrapper script.

-Peff

^ permalink raw reply

* Re: [PATCH] completion: Replace config --list with --get-regexp
From: Bert Wesarg @ 2009-09-13 10:51 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Todd Zullinger, Jeff King, james bardin, git, Junio C Hamano
In-Reply-To: <20090912183139.GO1033@spearce.org>

On Sat, Sep 12, 2009 at 20:31, Shawn O. Pearce <spearce@spearce.org> wrote:
> Todd Zullinger <tmz@pobox.com> wrote:
>> James Bardin noted that the completion spewed warnings when no git
>> config file is present.  This is likely a bug to be fixed in git config,
>> but it's also a good excuse to simplify the completion code by using the
>> --get-regexp option as Jeff King pointed out.
>>
>> Signed-off-by: Todd Zullinger <tmz@pobox.com>
>
> Thanks, looks good.
I have not looked into this, but what about pushurl?

Bert

^ permalink raw reply

* [PATCH v3] preserve mtime of local clone
From: Clemens Buchacher @ 2009-09-13 10:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, msysgit, Shawn O. Pearce
In-Reply-To: <7vvdjn4k6s.fsf@alter.siamese.dyndns.org>

A local clone without hardlinks copies all objects, including dangling
ones, to the new repository. Since the mtimes are renewed, those
dangling objects cannot be pruned by "git gc --prune", even if they
would have been old enough for pruning in the original repository.

Instead, preserve mtime during copy. "git gc --prune" will then work
in the clone just like it did in the original.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---

On Sat, Sep 12, 2009 at 08:06:51PM -0700, Junio C Hamano wrote:

> This new feature is not wanted by most of the callers of copy_file(), and
> it is not like they may want to conditionally pass 1 in preserve_times
> someday.  I'd limit the damage like this replacement patch, instead.

You're right. That's much better.

> Is it unreasonable to have a test for this one, by the way?

It's not. Test added.

Clemens

 builtin-clone.c  |    2 +-
 cache.h          |    2 ++
 copy.c           |   21 +++++++++++++++++++++
 t/t5304-prune.sh |   54 +++++++++++++++++++++++++++++++++---------------------
 4 files changed, 57 insertions(+), 22 deletions(-)

diff --git a/builtin-clone.c b/builtin-clone.c
index ad04808..bab2d84 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -269,7 +269,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest)
 				die_errno("failed to create link '%s'", dest->buf);
 			option_no_hardlinks = 1;
 		}
-		if (copy_file(dest->buf, src->buf, 0666))
+		if (copy_file_with_time(dest->buf, src->buf, 0666))
 			die_errno("failed to copy file to '%s'", dest->buf);
 	}
 	closedir(dir);
diff --git a/cache.h b/cache.h
index 5fad24c..ee9c98c 100644
--- a/cache.h
+++ b/cache.h
@@ -923,6 +923,8 @@ extern const char *git_mailmap_file;
 extern void maybe_flush_or_die(FILE *, const char *);
 extern int copy_fd(int ifd, int ofd);
 extern int copy_file(const char *dst, const char *src, int mode);
+extern int copy_times(const char *dst, const char *src);
+extern int copy_file_with_time(const char *dst, const char *src, int mode);
 extern ssize_t read_in_full(int fd, void *buf, size_t count);
 extern ssize_t write_in_full(int fd, const void *buf, size_t count);
 extern void write_or_die(int fd, const void *buf, size_t count);
diff --git a/copy.c b/copy.c
index e54d15a..963b481 100644
--- a/copy.c
+++ b/copy.c
@@ -55,3 +55,24 @@ int copy_file(const char *dst, const char *src, int mode)
 
 	return status;
 }
+
+int copy_times(const char *dst, const char *src)
+{
+	struct stat st;
+	struct utimbuf times;
+	if (stat(src, &st) < 0)
+		return -1;
+	times.actime = st.st_atime;
+	times.modtime = st.st_mtime;
+	if (utime(dst, &times) < 0)
+		return -1;
+	return 0;
+}
+
+int copy_file_with_time(const char *dst, const char *src, int mode)
+{
+     int status = copy_file(dst, src, mode);
+     if (!status)
+		return copy_times(dst, src);
+     return status;
+}
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index 55ed7c7..15fd90a 100755
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -6,6 +6,17 @@
 test_description='prune'
 . ./test-lib.sh
 
+day=$((60*60*24))
+week=$(($day*7))
+
+add_blob() {
+	before=$(git count-objects | sed "s/ .*//") &&
+	BLOB=$(echo aleph_0 | git hash-object -w --stdin) &&
+	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
+	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE
+}
+
 test_expect_success setup '
 
 	: > file &&
@@ -31,11 +42,7 @@ test_expect_success 'prune stale packs' '
 
 test_expect_success 'prune --expire' '
 
-	before=$(git count-objects | sed "s/ .*//") &&
-	BLOB=$(echo aleph | git hash-object -w --stdin) &&
-	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
-	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
-	test -f $BLOB_FILE &&
+	add_blob &&
 	git prune --expire=1.hour.ago &&
 	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
 	test -f $BLOB_FILE &&
@@ -48,16 +55,12 @@ test_expect_success 'prune --expire' '
 
 test_expect_success 'gc: implicit prune --expire' '
 
-	before=$(git count-objects | sed "s/ .*//") &&
-	BLOB=$(echo aleph_0 | git hash-object -w --stdin) &&
-	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
-	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
-	test -f $BLOB_FILE &&
-	test-chmtime =-$((86400*14-30)) $BLOB_FILE &&
+	add_blob &&
+	test-chmtime =-$((2*$week-30)) $BLOB_FILE &&
 	git gc &&
 	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
 	test -f $BLOB_FILE &&
-	test-chmtime =-$((86400*14+1)) $BLOB_FILE &&
+	test-chmtime =-$((2*$week+1)) $BLOB_FILE &&
 	git gc &&
 	test $before = $(git count-objects | sed "s/ .*//") &&
 	! test -f $BLOB_FILE
@@ -114,12 +117,8 @@ test_expect_success 'prune: do not prune heads listed as an argument' '
 
 test_expect_success 'gc --no-prune' '
 
-	before=$(git count-objects | sed "s/ .*//") &&
-	BLOB=$(echo aleph_0 | git hash-object -w --stdin) &&
-	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
-	test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
-	test -f $BLOB_FILE &&
-	test-chmtime =-$((86400*5001)) $BLOB_FILE &&
+	add_blob &&
+	test-chmtime =-$((5001*$day)) $BLOB_FILE &&
 	git config gc.pruneExpire 2.days.ago &&
 	git gc --no-prune &&
 	test 1 = $(git count-objects | sed "s/ .*//") &&
@@ -140,9 +139,8 @@ test_expect_success 'gc respects gc.pruneExpire' '
 
 test_expect_success 'gc --prune=<date>' '
 
-	BLOB=$(echo aleph_0 | git hash-object -w --stdin) &&
-	BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") &&
-	test-chmtime =-$((86400*5001)) $BLOB_FILE &&
+	add_blob &&
+	test-chmtime =-$((5001*$day)) $BLOB_FILE &&
 	git gc --prune=5002.days.ago &&
 	test -f $BLOB_FILE &&
 	git gc --prune=5000.days.ago &&
@@ -150,4 +148,18 @@ test_expect_success 'gc --prune=<date>' '
 
 '
 
+test_expect_success 'gc: prune old objects after local clone' '
+
+	add_blob &&
+	test-chmtime =-$((2*$week+1)) $BLOB_FILE &&
+	git clone --no-hardlinks . aclone &&
+	cd aclone &&
+	test 1 = $(git count-objects | sed "s/ .*//") &&
+	test -f $BLOB_FILE &&
+	git gc --prune &&
+	test 0 = $(git count-objects | sed "s/ .*//") &&
+	! test -f $BLOB_FILE
+
+'
+
 test_done
-- 
1.6.5.rc0.164.g5f6b0

^ permalink raw reply related

* Re: git push --confirm ?
From: Junio C Hamano @ 2009-09-13 10:37 UTC (permalink / raw)
  To: Jeff King; +Cc: Owen Taylor, git, Colin Walters
In-Reply-To: <20090913093324.GB14438@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

>> I do not think this is an unreasonable option to have.  Just please don't
>> justify this change based on atomicity argument, but justify it as a mere
>> convenience feature.
>
> I don't agree. Making sure we use the _same_ <old-sha1> in the
> confirmation output we show to the user and in the ref update we send to
> the remote is critical for this to be safe.

OK.

You may be giving stale info to the user if somebody else is pushing from
sideways anyway, and the difference between a separate --dry-run and real
push when that happens is where and how the human waits.

With a separate --dry-run, the wait happens while the output is examined
offline.  The user may run "git log --oneline old...new" himself before
deciding to run the real push.

With --confirm, the wait happens while the --confirm waits for the human,
and perhaps the command does "git log --oneline old...new" as convenience.
While all this is happening, the TCP connection to the remote end is still
kept open.  We do not lock anything, but if somebody else pushed from
sideways, at the end of this session we would notice that, and the push
will be aborted.

This somewhat makes me worry about DoS point of view, but it does make it
somewhat safer.

I think the largest practical safety would come from the fact that this
would make it convenient (i.e. a single command "push --confirm") than
having to run two separate ones with manual inspection in between.  A
safety feature that is cumbersome to use won't add much to safety, as that
is unlikely to be used in the first place.

^ permalink raw reply

* GIT 1.6.5-rc1
From: Junio C Hamano @ 2009-09-13 10:14 UTC (permalink / raw)
  To: git

I've tagged 1.6.5-rc1.  Here is how the draft release notes for it looks
like.

Please switch to regression-hunting and bug-fixing mode now ;-)

GIT v1.6.5 Release Notes (draft)
================================

In git 1.7.0, which was planned to be the release after 1.6.5, "git
push" into a branch that is currently checked out will be refused by
default.

You can choose what should happen upon such a push by setting the
configuration variable receive.denyCurrentBranch in the receiving
repository.

Also, "git push $there :$killed" to delete the branch $killed in a remote
repository $there, when $killed branch is the current branch pointed at by
its HEAD, will be refused by default.

You can choose what should happen upon such a push by setting the
configuration variable receive.denyDeleteCurrent in the receiving
repository.

To ease the transition plan, the receiving repository of such a
push running this release will issue a big warning when the
configuration variable is missing.  Please refer to:

  http://git.or.cz/gitwiki/GitFaq#non-bare
  http://thread.gmane.org/gmane.comp.version-control.git/107758/focus=108007

for more details on the reason why this change is needed and the
transition plan.

Updates since v1.6.4
--------------------

(subsystems)

 * various updates to git-svn and gitweb.

(portability)

 * more improvements on mingw port.

 * mingw will also give FRSX as the default value for the LESS
   environment variable when the user does not have one.

(performance)

 * On major platforms, the system can be compiled to use with Linus's
   block-sha1 implementation of the SHA-1 hash algorithm, which
   outperforms the default fallback implementation we borrowed from
   Mozzilla.

 * Unnecessary inefficiency in deepening of a shallow repository has
   been removed.

 * The "git" main binary used to link with libcurl, which then dragged
   in a large number of external libraries.  When using basic plumbing
   commands in scripts, this unnecessarily slowed things down.  We now
   implement http/https/ftp transfer as a separate executable as we
   used to.

 * "git clone" run locally hardlinks or copies the files in .git/ to
   newly created repository.  It used to give new mtime to copied files,
   but this delayed garbage collection to trigger unnecessarily in the
   cloned repository.  We now preserve mtime for these files to avoid
   this issue.

(usability, bells and whistles)

 * Human writable date format to various options, e.g. --since=yesterday,
   master@{2000.09.17}, are taught to infer some omitted input properly.

 * A few programs gave verbose "advice" messages to help uninitiated
   people when issuing error messages.  An infrastructure to allow
   users to squelch them has been introduced, and a few such messages
   can be silenced now.

 * refs/replace/ hierarchy is designed to be usable as a replacement
   of the "grafts" mechanism, with the added advantage that it can be
   transferred across repositories.

 * "git am" learned to optionally ignore whitespace differences.

 * "git am" handles input e-mail files that has CRLF line endings sensibly.

 * "git am" learned "--scissors" option to allow you to discard early part
   of an incoming e-mail.

 * "git checkout", "git reset" and "git stash" learned to pick and
   choose to use selected changes you made, similar to "git add -p".

 * "git clone" learned a "-b" option to pick a HEAD to check out
   different from the remote's default branch.

 * "git commit --dry-run $args" is a new recommended way to ask "what would
   happen if I try to commit with these arguments."

 * "git commit --dry-run" and "git status" shows conflicted paths in a
   separate section to make them easier to spot during a merge.

 * "git cvsimport" now supports password-protected pserver access even
   when the password is not taken from ~/.cvspass file.

 * "git fast-export" learned --no-data option that can be useful when
   reordering commits and trees without touching the contents of
   blobs.

 * "git fast-import" has a pair of new front-end in contrib/ area.

 * "git init" learned to mkdir/chdir into a directory when given an
   extra argument (i.e. "git init this").

 * "git instaweb" optionally can use mongoose as the web server.

 * "git log --decorate" can optionally be told with --decorate=full to
   give the reference name in full.

 * "git merge" issued an unnecessarily scary message when it detected
   that the merge may have to touch the path that the user has local
   uncommitted changes to. The message has been reworded to make it
   clear that the command aborted, without doing any harm.

 * "git push" can be told to be --quiet.

 * "git push" pays attention to url.$base.pushInsteadOf and uses a URL
   that is derived from the URL used for fetching.

 * informational output from "git reset" that lists the locally modified
   paths is made consistent with that of "git checkout $another_branch".

 * "git submodule" learned to give submodule name to scripts run with
   "foreach" subcommand.

 * various subcommands to "git submodule" learned --recursive option.

 * "git submodule summary" learned --files option to compare the work
   tree vs the commit bound at submodule path, instead of comparing
   the index.

 * "git upload-pack", which is the server side support for "git clone" and
   "git fetch", can call a new post-upload-pack hook for statistics purposes.

(developers)

 * With GIT_TEST_OPTS="--root=/p/a/t/h", tests can be run outside the
   source directory; using tmpfs may give faster turnaround.

^ permalink raw reply

* [ANNOUNCE] GIT 1.6.4.3
From: Junio C Hamano @ 2009-09-13 10:12 UTC (permalink / raw)
  To: git

The latest maintenance release GIT 1.6.4.3 is available at the
usual places:

  http://www.kernel.org/pub/software/scm/git/

  git-1.6.4.3.tar.{gz,bz2}			(source tarball)
  git-htmldocs-1.6.4.3.tar.{gz,bz2}		(preformatted docs)
  git-manpages-1.6.4.3.tar.{gz,bz2}		(preformatted docs)

The RPM binary packages for a few architectures are found in:

  RPMS/$arch/git-*-1.6.4.3-1.fc9.$arch.rpm	(RPM)


GIT v1.6.4.3 Release Notes
==========================

Fixes since v1.6.4.2
--------------------

* "git clone" from an empty repository gave unnecessary error message,
  even though it did everything else correctly.

* "git cvsserver" invoked git commands via "git-foo" style, which has long
  been deprecated.

* "git fetch" and "git clone" had an extra sanity check to verify the
  presense of the corresponding *.pack file before downloading *.idx
  file by issuing a HEAD request.  Github server however sometimes
  gave 500 (Internal server error) response to HEAD even if a GET
  request for *.pack file to the same URL would have succeeded, and broke
  clone over HTTP from some of their repositories.  As a workaround, this
  verification has been removed (as it is not absolutely necessary).

* "git grep" did not like relative pathname to refer outside the current
  directory when run from a subdirectory.

* an error message from "git push" was formatted in a very ugly way.

* "git svn" did not quote the subversion user name correctly when
  running its author-prog helper program.

Other minor documentation updates are included.

^ permalink raw reply

* What's cooking in git.git (Sep 2009, #03; Sun, 13)
From: Junio C Hamano @ 2009-09-13 10:10 UTC (permalink / raw)
  To: git

What's cooking in git.git (Sep 2009, #03; Sun, 13)
--------------------------------------------------

Here are the topics that have been cooking.  Commits prefixed with '-' are
only in 'pu' while commits prefixed with '+' are in 'next'.  The ones
marked with '.' do not appear in any of the integration branches, but I am
still holding onto them.

In 1.7.0, we plan to correct handful of warts in the interfaces everybody
agrees that they were mistakes.  The resulting system may not be strictly
backward compatible.  Currently planeed changes are:

 * refuse push to update the checked out branch in a non-bare repo by
   default

   Make "git push" into a repository to update the branch that is checked
   out fail by default.  You can countermand this default by setting a
   configuration variable in the receiving repository.

   http://thread.gmane.org/gmane.comp.version-control.git/107758/focus=108007

 * refuse push to delete the current branch by default

   Make "git push $there :$killed" to delete the branch that is pointed at
   by its HEAD fail by default.  You can countermand this default by
   setting a configuration variable in the receiving repository.

   http://thread.gmane.org/gmane.comp.version-control.git/108862/focus=108936

 * git-send-email won't make deep threads by default

   Many people said that by default when sending more than 2 patches the
   threading git-send-email makes by default is hard to read, and they
   prefer the default be one cover letter and each patch as a direct
   follow-up to the cover letter.  You can countermand this by setting a
   configuration variable.

   http://article.gmane.org/gmane.comp.version-control.git/109790

 * git-status won't be "git-commit --dry-run" anymore

   http://thread.gmane.org/gmane.comp.version-control.git/125989/focus=125993

 * "git-diff -w --exit-code" will exit success if only differences it
   found are whitespace changes that are stripped away from the output.

   http://thread.gmane.org/gmane.comp.version-control.git/119731/focus=119751

During 1.6.5 cycle, 'next' will hold topics meant for 1.6.5 and 1.7.0.

I tagged and pushed out 1.6.5-rc1.  As far as I am concered, all the big
topics eligible for 1.6.5 are now in, except for possibly gfi-options
series from Sverre.  Updates to subsystems (svn, gitk, gui, and gitweb)
may still need to be merged in.

I've been hoping to keep this cycle short, and I'm also hoping to tag the
real 1.6.5 before I vanish for about a week and half later in the
month. so it looks like there may have to be a 1.6.6 to include the
leftover topics.

--------------------------------------------------
[Graduated to "master"]

* jc/merge-saner-messages (2009-09-07) 1 commit
 + merge-recursive: give less scary messages when merge did not start

* rc/maint-http-no-head-pack-check (2009-09-09) 1 commit.
 + http.c: remove verification of remote packs

* db/vcs-helper (2009-09-03) 16 commits
 + Use a clearer style to issue commands to remote helpers
 + Make the "traditionally-supported" URLs a special case
  (merged to 'next' on 2009-08-07 at f3533ba)
 + Makefile: install hardlinks for git-remote-<scheme> supported by libcurl if possible
 + Makefile: do not link three copies of git-remote-* programs
 + Makefile: git-http-fetch does not need expat
 + http-fetch: Fix Makefile dependancies
 + Add transport native helper executables to .gitignore
 + git-http-fetch: not a builtin
 + Use an external program to implement fetching with curl
 + Add support for external programs for handling native fetches

Up to the part that eject -lcurl from the main "git" binary cleanly are
now in 'master', with a few fix-ups from Jim Mayering.

* cb/maint-1.6.3-grep-relative-up (2009-09-05) 2 commits.
 + grep: accept relative paths outside current working directory
 + grep: fix exit status if external_grep() punts

* jk/unwanted-advices (2009-09-09) 2 commits
 + status: make "how to stage" messages optional
 + push: make non-fast-forward help message configurable

* jt/pushinsteadof (2009-09-07) 2 commits
 + Add url.<base>.pushInsteadOf: URL rewriting for push only
 + Wrap rewrite globals in a struct in preparation for adding another set

* pk/fast-import-tars (2009-09-03) 1 commit
 + import-tars: Allow per-tar author and commit message.

* pk/fast-import-dirs (2009-09-03) 1 commit
 + Add script for importing bits-and-pieces to Git.

--------------------------------------------------
[New Topics]

* db/vcs-helper-rest (2009-09-03) 6 commits
 - Allow helpers to report in "list" command that the ref is unchanged
 - Add support for "import" helper command
 - Add a config option for remotes to specify a foreign vcs
 - Allow programs to not depend on remotes having urls
 - Allow fetch to modify refs
 - Use a function to determine whether a remote is valid
 (this branch is used by jh/cvs-helper.)

This is not exactly new.  It holds the remainder of the db/vcs-helper
topic.

--------------------------------------------------
[Stalled]

* je/send-email-no-subject (2009-08-05) 1 commit
  (merged to 'next' on 2009-08-30 at b6455c2)
 + send-email: confirm on empty mail subjects

The existing tests to covers the positive case (i.e. as long as the user
says "yes" to the "do you really want to send this message that lacks
subject", the message is sent) of this feature, but the feature itself
needs its own test to verify the negative case (i.e. does it correctly
stop if the user says "no"?)

* jh/cvs-helper (2009-08-18) 8 commits
 - More fixes to the git-remote-cvs installation procedure
 - Fix the Makefile-generated path to the git_remote_cvs package in git-remote-cvs
 - Add simple selftests of git-remote-cvs functionality
 - git-remote-cvs: Remote helper program for CVS repositories
 - 2/2: Add Python support library for CVS remote helper
 - 1/2: Add Python support library for CVS remote helper
 - Basic build infrastructure for Python scripts
 - Allow helpers to request marks for fast-import
 (this branch uses db/vcs-helper-rest.)

Builds on db/vcs-helper.  There is a re-roll planned.

* ne/rev-cache (2009-09-07) 7 commits
 . support for commit grafts, slight change to general mechanism
 . support for path name caching in rev-cache
 . full integration of rev-cache into git, completed test suite
 . administrative functions for rev-cache, start of integration into git
 . support for non-commit object caching in rev-cache
 . basic revision cache system, no integration or features
 . man page and technical discussion for rev-cache

Replaced but I do not think this is ready for 'pu' yet.

--------------------------------------------------
[Cooking]

* jh/notes (2009-09-12) 13 commits
 - Selftests verifying semantics when loading notes trees with various fanouts
 - Teach the notes lookup code to parse notes trees with various fanout schemes
 - notes.[ch] fixup: avoid old-style declaration
 - Teach notes code to free its internal data structures on request.
 - Add '%N'-format for pretty-printing commit notes
 - Add flags to get_commit_notes() to control the format of the note string
 - t3302-notes-index-expensive: Speed up create_repo()
 - fast-import: Add support for importing commit notes
 - Teach "-m <msg>" and "-F <file>" to "git notes edit"
 - Add an expensive test for git-notes
 - Speed up git notes lookup
 - Add a script to edit/inspect notes
 - Introduce commit notes
 (this branch uses sr/gfi-options.)

Rerolled and queued.

* jn/gitweb-show-size (2009-09-07) 1 commit
 - gitweb: Add 'show-sizes' feature to show blob sizes in tree view

* lt/maint-traverse-trees-fix (2009-09-06) 1 commit.
 - Prepare 'traverse_trees()' for D/F conflict lookahead

Beginning of the fix to a rather nasty longstanding issue of merging trees
with ("a" "a-b"), ("a/b" "a-b") and just ("a-b"), but my reading of it is
that it is just the first step to demonstrate one-entry lookahead and not
a full solution yet.

* jc/maint-1.6.0-blank-at-eof (2009-09-05) 10 commits.
  (merged to 'next' on 2009-09-07 at 165dc3c)
 + core.whitespace: split trailing-space into blank-at-{eol,eof}
 + diff --color: color blank-at-eof
 + diff --whitespace=warn/error: fix blank-at-eof check
 + diff --whitespace=warn/error: obey blank-at-eof
 + diff.c: the builtin_diff() deals with only two-file comparison
 + apply --whitespace: warn blank but not necessarily empty lines at EOF
 + apply --whitespace=warn/error: diagnose blank at EOF
 + apply.c: split check_whitespace() into two
 + apply --whitespace=fix: detect new blank lines at eof correctly
 + apply --whitespace=fix: fix handling of blank lines at the eof

This started a bit late in the cycle, and I'd rather hold it back during
this feature freeze and push it out after 1.6.5 final.

* jn/gitweb-blame (2009-09-01) 5 commits
 - gitweb: Minify gitweb.js if JSMIN is defined
 - gitweb: Create links leading to 'blame_incremental' using JavaScript
  (merged to 'next' on 2009-09-07 at 3622199)
 + gitweb: Colorize 'blame_incremental' view during processing
 + gitweb: Incremental blame (using JavaScript)
 + gitweb: Add optional "time to generate page" info in footer

Ajax-y blame.

* sr/gfi-options (2009-09-06) 6 commits
  (merged to 'next' on 2009-09-07 at 5f6b0ff)
 + fast-import: test the new option command
 + fast-import: add option command
 + fast-import: test the new feature command
 + fast-import: add feature command
 + fast-import: put marks reading in it's own function
 + fast-import: put option parsing code in separate functions
 (this branch is used by jh/notes.)

Perhaps 1.6.5 material but I wasn't sure.

* nd/sparse (2009-08-20) 19 commits
 - sparse checkout: inhibit empty worktree
 - Add tests for sparse checkout
 - read-tree: add --no-sparse-checkout to disable sparse checkout support
 - unpack-trees(): ignore worktree check outside checkout area
 - unpack_trees(): apply $GIT_DIR/info/sparse-checkout to the final index
 - unpack-trees(): "enable" sparse checkout and load $GIT_DIR/info/sparse-checkout
 - unpack-trees.c: generalize verify_* functions
 - unpack-trees(): add CE_WT_REMOVE to remove on worktree alone
 - Introduce "sparse checkout"
 - dir.c: export excluded_1() and add_excludes_from_file_1()
 - excluded_1(): support exclude files in index
 - unpack-trees(): carry skip-worktree bit over in merged_entry()
 - Read .gitignore from index if it is skip-worktree
 - Avoid writing to buffer in add_excludes_from_file_1()
 - Teach Git to respect skip-worktree bit (writing part)
 - Teach Git to respect skip-worktree bit (reading part)
 - Introduce "skip-worktree" bit in index, teach Git to get/set this bit
 - Add test-index-version
 - update-index: refactor mark_valid() in preparation for new options

--------------------------------------------------
[For 1.7.0]

* jk/1.7.0-status (2009-09-05) 5 commits
 - docs: note that status configuration affects only long format
  (merged to 'next' on 2009-09-07 at 8a7c563)
 + commit: support alternate status formats
 + status: add --porcelain output format
 + status: refactor format option parsing
 + status: refactor short-mode printing to its own function
 (this branch uses jc/1.7.0-status.)

Gives the --short output format to post 1.7.0 "git commit --dry-run" that
is similar to that of post 1.7.0 "git status".

* jc/1.7.0-status (2009-09-05) 4 commits
  (merged to 'next' on 2009-09-06 at 19d4beb)
 + status: typo fix in usage
  (merged to 'next' on 2009-08-22 at b3507bb)
 + git status: not "commit --dry-run" anymore
 + git stat -s: short status output
 + git stat: the beginning of "status that is not a dry-run of commit"
 (this branch is used by jk/1.7.0-status.)

With this, "git status" is no longer "git commit --dry-run".

* jc/1.7.0-send-email-no-thread-default (2009-08-22) 1 commit
  (merged to 'next' on 2009-08-22 at 5106de8)
 + send-email: make --no-chain-reply-to the default

* jc/1.7.0-diff-whitespace-only-status (2009-08-30) 4 commits.
  (merged to 'next' on 2009-08-30 at 0623572)
 + diff.c: fix typoes in comments
  (merged to 'next' on 2009-08-27 at 81fb2bd)
 + Make test case number unique
  (merged to 'next' on 2009-08-02 at 9c08420)
 + diff: Rename QUIET internal option to QUICK
 + diff: change semantics of "ignore whitespace" options

This changes exit code from "git diff --ignore-whitespace" and friends
when there is no actual output.  It is a backward incompatible change, but
we could argue that it is a bugfix.

* jc/1.7.0-push-safety (2009-02-09) 2 commits
  (merged to 'next' on 2009-08-02 at 38b82fe)
 + Refuse deleting the current branch via push
 + Refuse updating the current branch in a non-bare repository via push

--------------------------------------------------
[I have been too busy to purge these]

* jc/log-tz (2009-03-03) 1 commit.
 - Allow --date=local --date=other-format to work as expected

Maybe some people care about this.  I dunno.

* jc/mailinfo-remove-brackets (2009-07-15) 1 commit.
 - mailinfo: -b option keeps [bracketed] strings that is not a [PATCH] marker

Maybe some people care about this.  I dunno.

* lt/read-directory (2009-05-15) 3 commits.
 . Add initial support for pathname conversion to UTF-8
 . read_directory(): infrastructure for pathname character set conversion
 . Add 'fill_directory()' helper function for directory traversal

* cc/sequencer-rebase-i (2009-08-28) 15 commits
 - rebase -i: use "git sequencer--helper --cherry-pick"
 - sequencer: add "--cherry-pick" option to "git sequencer--helper"
 - sequencer: add "do_commit()" and related functions working on "next_commit"
 - pick: libify "pick_help_msg()"
 - revert: libify cherry-pick and revert functionnality
 - rebase -i: use "git sequencer--helper --fast-forward"
 - sequencer: let "git sequencer--helper" callers set "allow_dirty"
 - sequencer: add "--fast-forward" option to "git sequencer--helper"
 - sequencer: add "do_fast_forward()" to perform a fast forward
 - rebase -i: use "git sequencer--helper --reset-hard"
 - sequencer: add "--reset-hard" option to "git sequencer--helper"
 - sequencer: add "reset_almost_hard()" and related functions
 - rebase -i: use "git sequencer--helper --make-patch"
 - sequencer: add "make_patch" function to save a patch
 - sequencer: add "builtin-sequencer--helper.c"

This was to be replaced with multiple bite-sized smaller topics.  We've
seen one such topic for "git reset" on the list.

^ permalink raw reply

* Notification/Des lots: Numéro 074/05/ZY369
From: Irish News Centre @ 2009-09-13  9:55 UTC (permalink / raw)


Vous avez gagné £ 750,000, envoyer des informations: Nom, âge, pays

^ permalink raw reply

* Re: [PATCH] reflog documentation: -n is an alias for --dry-run
From: Jeff King @ 2009-09-13  9:40 UTC (permalink / raw)
  To: Nelson Elhage; +Cc: git
In-Reply-To: <1252813314-14408-1-git-send-email-nelhage@mit.edu>

On Sat, Sep 12, 2009 at 11:41:54PM -0400, Nelson Elhage wrote:

>  static const char reflog_expire_usage[] =
> -"git reflog (show|expire) [--verbose] [--dry-run] [--stale-fix] [--expire=<time>] [--expire-unreachable=<time>] [--all] <refs>...";
> +"git reflog (show|expire) [--verbose] [-n | --dry-run] [--stale-fix] [--expire=<time>] [--expire-unreachable=<time>] [--all] <refs>...";

Really? I think "git reflog show -n" is not about dry-run at all...

-Peff

^ permalink raw reply

* Re: git push --confirm ?
From: Jeff King @ 2009-09-13  9:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Owen Taylor, git, Colin Walters
In-Reply-To: <7vvdjn8ymk.fsf@alter.siamese.dyndns.org>

On Sat, Sep 12, 2009 at 05:41:23PM -0700, Junio C Hamano wrote:

> > But what _would_ be useful is doing it atomically. You can certainly do
> > all three of those steps from within one "git push" invocation, and I
> > think that is enough without any protocol changes. The protocol already
> > sends for each ref a line like:
> >
> >   <old-sha1> <new-sha1> <ref>
> >
> > and receive-pack will not proceed with the update unless the <old-sha1>
> > matches what is about to be changed.
> 
> Be careful that using that information and doing things in one session
> won't give you atomicity in the sense that it may still fail after you
> said "yes that is what I want to push, really" to the confirmation
> question.

Of course, but that issue exists already. It is just that the window
between receiving the refs and then asking them to be updated is much
smaller when there is no human input in the loop (and since we haven't
actually _shown_ the list to the user, it appears atomic to them).

I think this type of atomicity is fine for this application. The point
of this is to err on the side of caution. So it is OK to say "Push
this?" and then after the user has confirmed say "Oops, somebody pushed
something else while we were waiting for your input. Try again." The
important thing is to not say "Push this?", have the user confirm that
what they are pushing over is OK, and then end up pushing over something
different (which is what can happen with separate push invocations).

The only way to get true atomicity across the confirmation and push
would be to take a lock at the beginning of the push session. Which is
too coarse-grained in the first place (it disallows simultaneous update
of unrelated refs), but would also require protocol updates.

> It does save you an extra connection, compared to separate invocations
> without and then with --dry-run, so it still is a plus.
> 
> I do not think this is an unreasonable option to have.  Just please don't
> justify this change based on atomicity argument, but justify it as a mere
> convenience feature.

I don't agree. Making sure we use the _same_ <old-sha1> in the
confirmation output we show to the user and in the ref update we send to
the remote is critical for this to be safe.

-Peff

^ permalink raw reply

* Re: Effectively tracing project contributions with git
From: Jeff King @ 2009-09-13  9:24 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Joseph Wakeling, Sverre Rabbelier, git
In-Reply-To: <20090913022843.GB26588@mit.edu>

On Sat, Sep 12, 2009 at 10:28:43PM -0400, Theodore Tso wrote:

> > I don't see any solution that doesn't see me browsing diffs -- there's
> > no metric that will solve the problem -- but if your stats work could
> > help me get an output of the form 'here are all the diffs on file X by
> > contributor Y in order of size, largest first' then I think it would
> > help a LOT.
> 
> This will display all of the diffs on file (pathname) XXX by contributor YYY:
> 
> 	git log -p --author=YYY XXX 
> 
> You might also find the diffstats useful:
> 
> 	git log --stat --author=YYY XXX
> 
> Or if you want *only* the diffstats for the file in question, you might try:
> 
> 	git log --stat --pretty=format: --author=YYY XXX | grep XXX

There is also the "--numstat" format which is a bit easier for parsing.
I think the "all diffs on file $X by contributor $Y, ordered by size"
would look like:

  git log -z --pretty=tformat:%H --numstat --author=$Y $X |
  perl -0ne '
    my ($commit) = /^([0-9a-f]{40})$/m;
    my ($lines_added) = /^(\d+)\s/m;
    print "$lines_added $commit\n";
  ' |
  sort -rn |
  cut -d ' ' -f2 |
  xargs -n 1 git show

-Peff

^ permalink raw reply

* Re: [PATCH] transport-helper.c: don't leak fdopen'd stream buffers
From: Jim Meyering @ 2009-09-13  7:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git list, Daniel Barkalow, Johannes Sixt
In-Reply-To: <7vtyz760lm.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Jim Meyering <jim@meyering.net> writes:
>
>> diff --git a/transport-helper.c b/transport-helper.c
>> index f57e84c..0bbd014 100644
>> --- a/transport-helper.c
>> +++ b/transport-helper.c
>> @@ -49,6 +49,7 @@ static struct child_process *get_helper(struct transport *transport)
>>  		if (!strcmp(buf.buf, "fetch"))
>>  			data->fetch = 1;
>>  	}
>> +	fclose (file);
>>  	return data->helper;
>>  }
>>
>> @@ -88,6 +89,7 @@ static int fetch_with_fetch(struct transport *transport,
>>  		if (strbuf_getline(&buf, file, '\n') == EOF)
>>  			exit(128); /* child died, message supplied already */
>>  	}
>> +	fclose (file);
>>  	return 0;
>>  }
>
> The callchain of fetch_with_fetch() looks like:
>
>     fetch_with_fetch()
>         helper = get_helper();
>         --> get_helper()
>             - start helper with start_command();
>             - read from helper->out until it sees an empty line;
>             - break out of the loop;
>         <-- return helper
>         - file = xfdopen(helper->out) to get another FILE on the fd
>         - read the rest of the output from helper->out via file
>
> It seems to me that the fclose() in get_helper() will close the underlying
> fd and would break the caller, no?

I confess that my sole test was to run "make test", which passed.
If the fd must live on, a slightly less invasive change would be to
xdup each descriptor just before each of the three xfdopen calls, e.g.,

-       file = xfdopen(helper->out, "r");
+       file = xfdopen(xdup(helper->out), "r");

> I think "struct helper_data" should get a new FILE* field and once
> somebody creates a FILE* out of its helper->out, that FILE* can be passed
> around without a new xfdopen().
>
> Or something like that.

That's probably best.

> Who is responsible for closing the underlying helper->out fd in the
> start_command() API, by the way?

^ permalink raw reply

* Re: [PATCH] use write_str_in_full helper to avoid literal string lengths
From: Jim Meyering @ 2009-09-13  7:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git list
In-Reply-To: <7veiqbbttg.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
...
> Thanks.  I agree with the reasoning you wrote outside the proposed log
> message.
>
> We usually do not write these bullet points (iow, we are not GNU) in our
> log message.  The names of the functions, call sites and files that are
> involved are something anybody can see from the patch text,
>
> I think the GNU convention was useful back when we were trapped in a
> system with non-atomic commits, where it was very hard to see what files
> were affected in a single logical changeset (i.e. CVS).
>
> Luckily, we graduated those dark ages.
>
> Instead, we prefer to have justifications (and methods), like what you
> wrote at the beginning of your message.  These are not something people
> can find in the patch text and they deserve to be recorded in the commit.

Heh ;-)
Old habits...
Here's that same commit with a better log message:

>From 7edbf0141c9e2ae2999f8b7919c938b9384240e2 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@redhat.com>
Date: Sat, 12 Sep 2009 09:56:13 +0200
Subject: [PATCH] use write_str_in_full helper to avoid literal string lengths

In 2d14d65ce7b136b0fb18dcf27e5caff67829f658,
I noticed two changes like this:

-	write_in_full(helper->in, "list\n", 5);
+
+	strbuf_addstr(&buf, "list\n");
+	write_in_full(helper->in, buf.buf, buf.len);
+	strbuf_reset(&buf);

Instead, let's define a new function, in cache.h:

    static inline ssize_t write_str_in_full(int fd, const char *str)
    {
           return write_in_full(fd, str, strlen(str));
    }

and then use it like this:

-       strbuf_addstr(&buf, "list\n");
-       write_in_full(helper->in, buf.buf, buf.len);
-       strbuf_reset(&buf);
+       write_str_in_full(helper->in, "list\n");

Thus not requiring the added allocation, and still avoiding
the maintenance risk of literal string lengths.
These days, compilers are good enough that strlen("literal")
imposes no run-time cost.

Update the two uses in transport-helper.c manually,
and transform the other uses via this:

    perl -pi -e \
        's/write_in_full\((.*?), (".*?"), \d+\)/write_str_in_full($1, $2)/'\
      $(git grep -l 'write_in_full.*"')

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 builtin-fetch.c    |    2 +-
 builtin-reflog.c   |    2 +-
 cache.h            |    9 +++++++--
 commit.c           |    2 +-
 config.c           |    2 +-
 rerere.c           |    2 +-
 transport-helper.c |   10 +++-------
 upload-pack.c      |    4 ++--
 8 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/builtin-fetch.c b/builtin-fetch.c
index 817dd6b..cb48c57 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -454,7 +454,7 @@ static int quickfetch(struct ref *ref_map)

 	for (ref = ref_map; ref; ref = ref->next) {
 		if (write_in_full(revlist.in, sha1_to_hex(ref->old_sha1), 40) < 0 ||
-		    write_in_full(revlist.in, "\n", 1) < 0) {
+		    write_str_in_full(revlist.in, "\n") < 0) {
 			if (errno != EPIPE && errno != EINVAL)
 				error("failed write to rev-list: %s", strerror(errno));
 			err = -1;
diff --git a/builtin-reflog.c b/builtin-reflog.c
index 95198c5..e23b5ef 100644
--- a/builtin-reflog.c
+++ b/builtin-reflog.c
@@ -362,7 +362,7 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
 		} else if (cmd->updateref &&
 			(write_in_full(lock->lock_fd,
 				sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
-			 write_in_full(lock->lock_fd, "\n", 1) != 1 ||
+			 write_str_in_full(lock->lock_fd, "\n") != 1 ||
 			 close_ref(lock) < 0)) {
 			status |= error("Couldn't write %s",
 				lock->lk->filename);
diff --git a/cache.h b/cache.h
index 30a7a16..90bf127 100644
--- a/cache.h
+++ b/cache.h
@@ -924,13 +924,18 @@ extern const char *git_mailmap_file;
 extern void maybe_flush_or_die(FILE *, const char *);
 extern int copy_fd(int ifd, int ofd);
 extern int copy_file(const char *dst, const char *src, int mode);
-extern ssize_t read_in_full(int fd, void *buf, size_t count);
-extern ssize_t write_in_full(int fd, const void *buf, size_t count);
 extern void write_or_die(int fd, const void *buf, size_t count);
 extern int write_or_whine(int fd, const void *buf, size_t count, const char *msg);
 extern int write_or_whine_pipe(int fd, const void *buf, size_t count, const char *msg);
 extern void fsync_or_die(int fd, const char *);

+extern ssize_t read_in_full(int fd, void *buf, size_t count);
+extern ssize_t write_in_full(int fd, const void *buf, size_t count);
+static inline ssize_t write_str_in_full(int fd, const char *str)
+{
+	return write_in_full(fd, str, strlen(str));
+}
+
 /* pager.c */
 extern void setup_pager(void);
 extern const char *pager_program;
diff --git a/commit.c b/commit.c
index a6c6f70..fedbd5e 100644
--- a/commit.c
+++ b/commit.c
@@ -212,7 +212,7 @@ int write_shallow_commits(int fd, int use_pack_protocol)
 			else {
 				if (write_in_full(fd, hex,  40) != 40)
 					break;
-				if (write_in_full(fd, "\n", 1) != 1)
+				if (write_str_in_full(fd, "\n") != 1)
 					break;
 			}
 		}
diff --git a/config.c b/config.c
index f21530c..c644061 100644
--- a/config.c
+++ b/config.c
@@ -1119,7 +1119,7 @@ int git_config_set_multivar(const char *key, const char *value,
 				    copy_end - copy_begin)
 					goto write_err_out;
 				if (new_line &&
-				    write_in_full(fd, "\n", 1) != 1)
+				    write_str_in_full(fd, "\n") != 1)
 					goto write_err_out;
 			}
 			copy_begin = store.offset[i];
diff --git a/rerere.c b/rerere.c
index 87360dc..29f95f6 100644
--- a/rerere.c
+++ b/rerere.c
@@ -61,7 +61,7 @@ static int write_rr(struct string_list *rr, int out_fd)
 		path = rr->items[i].string;
 		length = strlen(path) + 1;
 		if (write_in_full(out_fd, rr->items[i].util, 40) != 40 ||
-		    write_in_full(out_fd, "\t", 1) != 1 ||
+		    write_str_in_full(out_fd, "\t") != 1 ||
 		    write_in_full(out_fd, path, length) != length)
 			die("unable to write rerere record");
 	}
diff --git a/transport-helper.c b/transport-helper.c
index b1ea7e6..832d81f 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -37,9 +37,7 @@ static struct child_process *get_helper(struct transport *transport)
 		die("Unable to run helper: git %s", helper->argv[0]);
 	data->helper = helper;

-	strbuf_addstr(&buf, "capabilities\n");
-	write_in_full(helper->in, buf.buf, buf.len);
-	strbuf_reset(&buf);
+	write_str_in_full(helper->in, "capabilities\n");

 	file = fdopen(helper->out, "r");
 	while (1) {
@@ -58,7 +56,7 @@ static int disconnect_helper(struct transport *transport)
 {
 	struct helper_data *data = transport->data;
 	if (data->helper) {
-		write_in_full(data->helper->in, "\n", 1);
+		write_str_in_full(data->helper->in, "\n");
 		close(data->helper->in);
 		finish_command(data->helper);
 		free((char *)data->helper->argv[0]);
@@ -124,9 +122,7 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)

 	helper = get_helper(transport);

-	strbuf_addstr(&buf, "list\n");
-	write_in_full(helper->in, buf.buf, buf.len);
-	strbuf_reset(&buf);
+	write_str_in_full(helper->in, "list\n");

 	file = fdopen(helper->out, "r");
 	while (1) {
diff --git a/upload-pack.c b/upload-pack.c
index c77ab71..b3471e4 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -553,7 +553,7 @@ static void receive_needs(void)

 	shallow_nr = 0;
 	if (debug_fd)
-		write_in_full(debug_fd, "#S\n", 3);
+		write_str_in_full(debug_fd, "#S\n");
 	for (;;) {
 		struct object *o;
 		unsigned char sha1_buf[20];
@@ -619,7 +619,7 @@ static void receive_needs(void)
 		}
 	}
 	if (debug_fd)
-		write_in_full(debug_fd, "#E\n", 3);
+		write_str_in_full(debug_fd, "#E\n");

 	if (!use_sideband && daemon_mode)
 		no_progress = 1;
--
1.6.5.rc0.190.g15871

^ permalink raw reply related

* Re: merge ignores --no-commit in fast-forward case
From: Junio C Hamano @ 2009-09-13  7:30 UTC (permalink / raw)
  To: Tomas Carnecky; +Cc: git mailing list
In-Reply-To: <A754ACF1-77C2-4036-A15C-8949E76BD2D5@dbservice.com>

Tomas Carnecky <tom@dbservice.com> writes:

> Bug or feature?

Feature, perhaps a misleading one.  As a fast-forward merge does not _create_
a commit, and --no-commit is about not creating a commit, it is logically
consistent.  But it is not useful nor intuitive to be logically consistent
in this case.

> Three possible solutions that I see:
>
>  2) Refuse to do anything if user gives --no-commit and merge is fast- 
> forward
>  3) Document this behavior in the manpage
>  4) Quietly set deny_non_fast_forward when --no-commit was given

Heh, this is new.  People laugh at me when I number my bullets starting
from zero, like all good computer scientists do ;-)

Seriously, we should at least do #3, and as a backward incompatible change
at least _consider_ doing #2 (I think #4 is merely an implementation detail
of #2), and if list reaches concensus in favor of such a change, come up
with a transition plan and do so in the 1.7.0 release.

^ permalink raw reply

* merge ignores --no-commit in fast-forward case
From: Tomas Carnecky @ 2009-09-13  6:40 UTC (permalink / raw)
  To: git mailing list

Bug or feature?

Three possible solutions that I see:

  2) Refuse to do anything if user gives --no-commit and merge is fast- 
forward
  3) Document this behavior in the manpage
  4) Quietly set deny_non_fast_forward when --no-commit was given

A fourth solution would be to not change anything, but users usually  
have a reason for using --no-commit, and if git commits behind their  
backs anyway that's just not nice.

tom

^ permalink raw reply

* [PATCH] Improve --patch option documentation in git-add
From: Jari Aalto @ 2009-09-13  6:44 UTC (permalink / raw)
  To: git
In-Reply-To: <7vmy5fy2hz.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Jari Aalto <jari.aalto@cante.net> writes:
>
>>     --patch:
>>     -p::
>>         In a modified work tree, choose interactively which patch hunks to
>>         add. This gives a change to review the difference between the
>>         index and the work before adding modified contents to the index.
>
> Sounds sensible.  You may want to be even more direct and succinct, e.g.
>
>     Interactively choose hunks of patch between the index and the work
>     tree and add them to the index.

Thanks, see below,
Jari

>From 63aa94e7782d6340ead0446ea80ed6223d7ac5c1 Mon Sep 17 00:00:00 2001
From: Jari Aalto <jari.aalto@cante.net>
Date: Sun, 13 Sep 2009 09:43:10 +0300
Subject: [PATCH] Improve --patch option documentation in git-add

Signed-off-by: Jari Aalto <jari.aalto@cante.net>
---
 Documentation/git-add.txt |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index e67b7e8..b94fbec 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -72,9 +72,14 @@ OPTIONS
 
 -p::
 --patch::
-	Similar to Interactive mode but the initial command loop is
-	bypassed and the 'patch' subcommand is invoked using each of
-	the specified filepatterns before exiting.
+	Interactively choose hunks of patch between the index and the
+	work tree and add them to the index. This gives a change to
+	review the difference before adding modified contents to the
+	index.
+
+	This effectively runs ``add --interactive``, but bypass the
+	initial command menu and directly jump to `patch` subcommand.
+	See ``Interactive mode'' for details.
 
 -e, \--edit::
 	Open the diff vs. the index in an editor and let the user
-- 
1.6.3.3

^ permalink raw reply related

* Re: [PATCH] git-push: Accept -n as a synonym for --dry-run.
From: Junio C Hamano @ 2009-09-13  6:10 UTC (permalink / raw)
  To: Nelson Elhage; +Cc: Junio C Hamano, git
In-Reply-To: <7vd45v2za4.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> ..., so you are not arguing against
> me at all.
> ... it is likely that we would try very
> hard to find a name that does not begin with 'n' to avoid the issue.

So what's my objection, in short?

It is just the wording "_the_ standard" that implies that anything that
uses -n to mean something other than --dry-run is _wrong_.

I didn't want to see anybody arguing against "commit -n".  It is used to
defeat pre-commit hook and has been there with us for a long time.

^ permalink raw reply

* Re: [RFC/PATCH v4 2/2] gitweb: append short hash ids to snapshot files
From: Junio C Hamano @ 2009-09-13  5:57 UTC (permalink / raw)
  To: Mark Rada; +Cc: git, Jakub Narebski
In-Reply-To: <4AAC85AC.9080004@mailservices.uwaterloo.ca>

Mark Rada <marada@uwaterloo.ca> writes:

> This was a manifestation of a suggestion from Jakub:
>
>> Second, I'd rather have better names for snapshots than using full SHA-1.
>> For snapshot of 'v1.5.0' of repository 'repo.git' I'd prefer for snapshot
>> to be named 'repo-v1.5.0', and for snapshot of 'next' branch of the same

I think I've already said "Don't use $full_hash but if the user gave you
descriptive e.g. v1.5.0 in $hash just use it", which I think matches what
Jakub said above.

>> project to be named for example 'repo-next-20090909', or perhaps
>> 'repo-next-2009-09-10T09:16:18' or 'repo-next-20090909-g5f6b0ff',
>> or 'repo-v1.6.5-rc0-164-g5f6b0ff'.

I do not particularly care about these, except that if the user asked for
'next', that string should be in the name somewhere, so the last one is
unnacceptable to me.  I'd rather vote for naming it just 'repo-next', as
if I were writing a robot that goes once-a-day to next and download, I
would likely to be doing it like this:

	D=`date +'%Y-%m-%d'` && mkdir "$D" && cd "$D" || exit
        wget ...snapshot-url-for-next-branch...
	wget ...snapshot-url-for-some-other-branch...
	wget ...snapshot-url-for-even-some-other-repository...
	...

and I do not want any frills other than what I _asked_ gitweb to give me,
which is "this repository, this branch".

But that is just my personal preference.  Treat it as no heavier than a
feature request from a random list participant.  It does not carry any
more weight than that merely because it comes from me.

> For me, there are two fates that snapshots will end up with: being deleted
> as soon as I have unrolled the contents, or long term archiving. For the
> latter case, it is nice to have an idea of when it came from, though I
> guess I should have appended a date in that case... ¯\(°_o)/¯

What date do you mean?  The commit date?  Or download date?

As long as it is clear which revision the snapshot came from, I do not
think anything fancier is necessary.

Besides, don't the paths in the archive have the timestamp of the commit
object?

If you are talking about download date for archival use, I think the
timestamp of the archive file itself is sufficient, and the person who is
downloading can (re)name the result in whatever way he wants.

^ permalink raw reply

* Re: [RFC/PATCH v4 1/2] gitweb: check given hash before trying to create snapshot
From: Junio C Hamano @ 2009-09-13  5:42 UTC (permalink / raw)
  To: Mark Rada; +Cc: Junio C Hamano, git, Jakub Narebski
In-Reply-To: <4AAC8521.1060005@mailservices.uwaterloo.ca>

Mark Rada <marada@uwaterloo.ca> writes:

> On 09-09-12 11:30 PM, Junio C Hamano wrote:
>>> @@ -5196,8 +5202,9 @@ sub git_snapshot {
>>>  		die_error(403, "Unsupported snapshot format");
>>>  	}
>>>  
>>> -	if (!defined $hash) {
>>> -		$hash = git_get_head_hash($project);
>>> +	my $full_hash = git_get_full_hash($project, $hash);
>>> +	if (!$full_hash) {
>>> +		die_error(404, 'Hash id was not valid');
>>>  	}
>> 
>> This is in the context of "snapshot", so obviously you care more about
>> just "such an object exists", don't you?  You also want it to be a
>> tree-ish.  Try giving it $hash = 'junio-gpg-pub' and see how it breaks.
>  
> You have confused me. How is using 'junio-gpg-pub' different from the 
> second test case that tries to use 'frizzumFrazzum'?

junio-gpg-pub tag exists in git.git but it tags a blob not a tree.

	$ git rev-parse junio-gpg-pub
        6019c27d966fe3ce8adcc0e9f12078eef96ca6ef
        $ git archive junio-gpg-pub
        fatal: not a tree object

^ permalink raw reply

* Re: [RFC/PATCH v4 2/2] gitweb: append short hash ids to snapshot files
From: Mark Rada @ 2009-09-13  5:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Mark Rada, git, Jakub Narebski
In-Reply-To: <7v7hw34ivl.fsf@alter.siamese.dyndns.org>

On 09-09-12 11:35 PM, Junio C Hamano wrote:
>> @@ -5207,6 +5227,12 @@ sub git_snapshot {
>> ...
>> +
>> +	if ($full_hash !~ /$hash/) {
>> +		$hash .= '-' . git_get_short_hash($project, $hash);
>> +	} else {
>> +		$hash = git_get_short_hash($project, $hash);
>> +	}
> 
> I do not get this test.  What is this unanchored pattern match about?

I missed that, it should have been anchored.

> I do not think you wanted to allow matching a partial 1234567 $hash to
> substitute a full 01234567..... $full_hash, so I am guessing that you
> meant to say "$full_hash !~ /^$hash/" at least, or perhaps you meant even
> "$full_hash ne $hash".
> 
> But that still does not make much sense to me.  Perhaps you meant to catch
> a case where $hash is a tagname (or refname), i.e. $hash = 'v1.6.3' or
> $hash = 'next'?
> 
> If that is indeed the case, then perhaps you should check for that more
> explicitly, perhaps using "git show-ref $hash" or something.  I do not
> know if the complexity (not just the "detect handcrafted $hash string that
> is not an SHA-1", but this whole "give shorten one" topic) is worth it,
> though.  And if you drop the hunk that changes user supplied $hash to
> $full_hash in the output file name in your [PATCH 1/2], I do not think you
> need this anyway.  If somebody asked for 'next', he will get 'next'.
> 
> If somebody asked for 01234... (full 40 hexdigits) because that was the
> link on the gitweb output page, it might make sense to give him a
> shortened name, but then the above conditional needs to be only:
> 
> 	if ($full_hash eq $hash) {
>         	$hash = git_get_short_hash($project, $hash);
> 	}
> 
> no?

This was a manifestation of a suggestion from Jakub:
> Second, I'd rather have better names for snapshots than using full SHA-1.
> For snapshot of 'v1.5.0' of repository 'repo.git' I'd prefer for snapshot
> to be named 'repo-v1.5.0', and for snapshot of 'next' branch of the same
> project to be named for example 'repo-next-20090909', or perhaps
> 'repo-next-2009-09-10T09:16:18' or 'repo-next-20090909-g5f6b0ff',
> or 'repo-v1.6.5-rc0-164-g5f6b0ff'.
> 
> I'm not sure what would be the best name of snapshot of given 
> subdirectory...
> 
> 
> In short: I'd rather not improve on bad design of using full SHA-1
> in snapshot name.

For me, there are two fates that snapshots will end up with: being deleted
as soon as I have unrolled the contents, or long term archiving. For the
latter case, it is nice to have an idea of when it came from, though I
guess I should have appended a date in that case... ¯\(°_o)/¯

Thoughts?


>> +test_commit \
>> +	'SnapshotFileTests' \
>> +	'i can has snapshot?'
>> +test_expect_success \
>> +	'snapshots: give full hash' \
>> +	'ID=`git rev-parse --verify HEAD` &&
>> +	gitweb_run "p=.git;a=snapshot;h=$ID;sf=tgz" &&
>> +	ID=`git rev-parse --short HEAD` &&
>> +	grep ".git-$ID.tar.gz" gitweb.output'
> 
> I'd rather see these indented like:
> 
>         test_expect_success 'snapshots: give full hash' '
> 		ID=$(git rev-parse --verify HEAD) &&
> 		gitweb_run ...
>         '
> 
> Also, if I am not mistaken, "test_commit" is not about doing any test, but
> is a short-hand for doing an operation, right?  It would be better to have
> it inside test_expect_success just in case your "git commit" or some other
> commands are broken.  I.e. like
> 
> 	test_expect_success 'create a test commit' '
> 		test_commit SnapshotFileTests "Can I have shapshot?"
>         '

Can I have snapshot?!?! What do you have against LOLspeak? :P


-- 
Mark Rada (ferrous26)
marada@uwaterloo.ca

^ permalink raw reply

* Re: [RFC/PATCH v4 1/2] gitweb: check given hash before trying to create snapshot
From: Mark Rada @ 2009-09-13  5:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jakub Narebski
In-Reply-To: <7vmy4z4j35.fsf@alter.siamese.dyndns.org>

On 09-09-12 11:30 PM, Junio C Hamano wrote:
>> @@ -5196,8 +5202,9 @@ sub git_snapshot {
>>  		die_error(403, "Unsupported snapshot format");
>>  	}
>>  
>> -	if (!defined $hash) {
>> -		$hash = git_get_head_hash($project);
>> +	my $full_hash = git_get_full_hash($project, $hash);
>> +	if (!$full_hash) {
>> +		die_error(404, 'Hash id was not valid');
>>  	}
> 
> This is in the context of "snapshot", so obviously you care more about
> just "such an object exists", don't you?  You also want it to be a
> tree-ish.  Try giving it $hash = 'junio-gpg-pub' and see how it breaks.
 
You have confused me. How is using 'junio-gpg-pub' different from the 
second test case that tries to use 'frizzumFrazzum'?


-- 
Mark Rada (ferrous26)
marada@uwaterloo.ca

^ permalink raw reply

* Re: [PATCH] git-push: Accept -n as a synonym for --dry-run.
From: Junio C Hamano @ 2009-09-13  5:23 UTC (permalink / raw)
  To: Nelson Elhage; +Cc: Junio C Hamano, git
In-Reply-To: <20090913035421.GP4275@mit.edu>

Nelson Elhage <nelhage@MIT.EDU> writes:

> ... I think my
> general argument still stands for commands where that is not the case.

Cool down.

It is a mere subset of what I already said, so you are not arguing against
me at all.

    So the justification should be more like "push does not have any other
    option that deserves a short-and-sweet -n better, it will not have any
    such option in the future, and --dry-run is very often used that it
    deserves to use -n as its short-hand."

and I already said I tend to agree with the first two points.  Indeed the
first point is an absolute truth (the statement is about the current
state).

To answer the second point you need to look into the future, but I do not
foresee us adding a very useful option to the command whose usefulness far
outweigh that of dry-run and whose name begins with 'n' to want to use it
as the short-hand.  In such a case, it is likely that we would try very
hard to find a name that does not begin with 'n' to avoid the issue.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox