Git development
 help / color / mirror / Atom feed
* Re: [PATCH] fix multiple issues in index-pack
From: Junio C Hamano @ 2008-10-20 21:31 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git
In-Reply-To: <alpine.LFD.2.00.0810201609300.26244@xanadu.home>

Nicolas Pitre <nico@cam.org> writes:

> Damn... this one was subtle.  And I'm still wondering how the hell the 
> test suite is able to pass with this.  I'll try to figure out why and 
> come up with better tests.

Thanks; much appreciated.

^ permalink raw reply

* Re: Feedback outside of the user survey
From: Junio C Hamano @ 2008-10-20 22:41 UTC (permalink / raw)
  To: Christian Jaeger; +Cc: Andreas Ericsson, Garry Dolley, Richard Hartmann, git
In-Reply-To: <48FCB87B.1080207@jaeger.mine.nu>

Christian Jaeger <christian@jaeger.mine.nu> writes:

>> That's partially implemented, I think (google for Nguy (or something, I'm
>> not very god with asian names),
>
> That's not enough information for me to find what you've had in
> mind. "stump Nguy site:marc.info" doesn't yield a result with Google.

I think Andreas is referring to nd/narrow topic currently parked in 'pu'.

$ git log next..36aa66d^2

^ permalink raw reply

* Re: gitk: Turn short SHA1 names into links too
From: Paul Mackerras @ 2008-10-20 23:20 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.1.10.0809251657080.3265@nehalem.linux-foundation.org>

Linus Torvalds writes:

> And the thing I wanted to work was to have the abbreviated SHA1's that 
> have started to get more common in the kernel commit logs work as links in 
> gitk too, just the way a full 40-character SHA1 link works.

I just pushed out a commit to gitk that makes this work, and fixes the
other bugs you mentioned.

Paul.

^ permalink raw reply

* Re: [GITK PATCH v2] Add menu accelerators
From: Paul Mackerras @ 2008-10-20 23:21 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <200810200000.33035.robin.rosenberg@dewire.com>

Robin Rosenberg writes:

> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> ---
>  gitk |   80 +++++++++++++++++++++++++++++++++---------------------------------
>  1 files changed, 40 insertions(+), 40 deletions(-)
> 
> This one is slightly better than the first one.  Works with gitk id...id too.

With that patch, I'm finding that pressing Alt-f pops up the File
menu, but also triggers the binding for 'f' in the main window (scroll
diff window to the next file), which is annoying.  I'm not sure why, I
will need to track that down.

Paul.

^ permalink raw reply

* Re: [PATCH] rebase-i-p: only list commits that require rewriting in todo
From: Junio C Hamano @ 2008-10-20 23:36 UTC (permalink / raw)
  To: Jeff King; +Cc: Stephen Haberman, gitster, git
In-Reply-To: <20081020115003.GA11309@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Wed, Oct 15, 2008 at 02:44:38AM -0500, Stephen Haberman wrote:
>
>> +					cat "$TODO" | grep -v "${rev:0:7}" > "${TODO}2" ; mv "${TODO}2" "$TODO"
>
> Substring expansion (like ${rev:0:7}) is not portable. At least it
> doesn't work on FreeBSD /bin/sh, and "it's not even in POSIX", I
> believe.

True.

I do not remember the individual patches in the series, but I have to say
that the script at the tip of the topic is, eh, less than ideal.

Here is a small untested patch to fix a few issues I spotted while reading
it for two minutes.

 * Why filter output from "rev-list --left-right A...B" and look for the
   ones that begin with ">"?  Wouldn't "rev-list A..B" give that?

 * The abbreviated SHA-1 are made with "rev-list --abbrev=7" into $TODO in
   an earlier invocation, and it can be more than 7 letters to avoid
   ambiguity.  Not just that "${r:0:7} is not even in POSIX", but use of
   it here is actively wrong.

 * There is no point in catting a single file and piping it into grep.


 git-rebase--interactive.sh |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git i/git-rebase--interactive.sh w/git-rebase--interactive.sh
index 848fbe7..a563dea 100755
--- i/git-rebase--interactive.sh
+++ w/git-rebase--interactive.sh
@@ -635,8 +635,8 @@ first and then run 'git rebase --continue' again."
 				sed -n "s/^>//p" > "$DOTEST"/not-cherry-picks
 			# Now all commits and note which ones are missing in
 			# not-cherry-picks and hence being dropped
-			git rev-list $UPSTREAM...$HEAD --left-right | \
-				sed -n "s/^>//p" | while read rev
+			git rev-list $UPSTREAM..$HEAD |
+			while read rev
 			do
 				if test -f "$REWRITTEN"/$rev -a "$(grep "$rev" "$DOTEST"/not-cherry-picks)" = ""
 				then
@@ -645,7 +645,8 @@ first and then run 'git rebase --continue' again."
 					# just the history of its first-parent for others that will
 					# be rebasing on top of it
 					git rev-list --parents -1 $rev | cut -d' ' -f2 > "$DROPPED"/$rev
-					cat "$TODO" | grep -v "${rev:0:7}" > "${TODO}2" ; mv "${TODO}2" "$TODO"
+					short=$(git rev-list -1 --abbrev-commit --abbrev=7 $rev)
+					grep -v "^[a-z][a-z]* $short" <"$TODO" > "${TODO}2" ; mv "${TODO}2" "$TODO"
 					rm "$REWRITTEN"/$rev
 				fi
 			done

^ permalink raw reply related

* Re: [PATCH, RFC] diff: add option to show context between close chunks
From: Junio C Hamano @ 2008-10-20 23:43 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git Mailing List, Davide Libenzi
In-Reply-To: <48FB757B.9030105@lsrfire.ath.cx>

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> I think it makes sense to make 1, or even 3, the default for this
> option for all commands that create patches intended for human
> consumption.  The patch keeps the default at 0, though.

I think defaulting to 1 would make sense, or alternatively, just
hardcoding that behaviour without any new option.  That would give you
more information with the same number of patch lines, iow, upside without
any downside.

diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index d3d9c84..3bf2581 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -60,9 +60,9 @@ static int xdl_emit_record(xdfile_t *xdf, 
  */
 static xdchange_t *xdl_get_hunk(xdchange_t *xscr, xdemitconf_t const *xecfg) {
 	xdchange_t *xch, *xchp;
 
 	for (xchp = xscr, xch = xscr->next; xch; xchp = xch, xch = xch->next)
-		if (xch->i1 - (xchp->i1 + xchp->chg1) > 2 * xecfg->ctxlen)
+		if (xch->i1 - (xchp->i1 + xchp->chg1) > 2 * xecfg->ctxlen + 1)
 			break;
 
 	return xchp;

^ permalink raw reply related

* Re: git add --patch newfile doesn't add newfile to cache ?
From: Jeff King @ 2008-10-20 23:50 UTC (permalink / raw)
  To: Marc Weber; +Cc: git
In-Reply-To: <20081020143636.GB3988@gmx.de>

On Mon, Oct 20, 2008 at 04:36:36PM +0200, Marc Weber wrote:

> Is this desired behaviour?
> [...]
>         git init
>         echo test > test
>         git add --patch test
>         echo "running status, nothing has been added"
>         git status

I think your example makes sense, but nobody ever really tried it
before. I use "git add -p" all the time, but almost always when I am
adding a new file, I add the whole contents.

I think there are two ways to go about fixing it:

  - in git-add--interactive.perl, the function patch_update_cmd
    explicitly looks at the list of modified files. It would have to
    also check for untracked files, which is easy. But we also need to
    keep track of which files are modified and which are untracked
    through the whole patching procedure, which is a bit more invasive.

  - the recently-added "git add -N" adds an empty file into the index,
    at which point we could add content in the normal way. So:

      git add -N test
      git add -p test

    should just work (but obviously requires two steps from the user).
    You could do something more automatic like the patch below, but I
    think the semantics aren't quite right. If you stage nothing for a
    newly added file, then you still end up with an empty version of the
    staged file in the index. I would expect the semantics to be:

      1. if you stage any content, then the file is added to the index
         with that content

      2. if you stage no content, then the file remains untracked

---
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index da768ee..72f8a67 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -811,6 +811,12 @@ EOF
 }
 
 sub patch_update_cmd {
+	my @new = list_untracked();
+	if (@new) {
+		system(qw(git add -N), @new)
+			and die "git add reported failure";
+	}
+
 	my @mods = grep { !($_->{BINARY}) } list_modified('file-only');
 	my @them;
 

^ permalink raw reply related

* Re: [PATCH] Teach/Fix -q/-v to pull/fetch
From: Junio C Hamano @ 2008-10-20 23:52 UTC (permalink / raw)
  To: Tuncer Ayaz; +Cc: git
In-Reply-To: <4ac8254d0810191133v79ed73b7tf09a282f44d302dd@mail.gmail.com>

"Tuncer Ayaz" <tuncer.ayaz@gmail.com> writes:

> it doesn't work as I expected when you supply -q and -v.
> I have to redo it, I guess.

Probably adding a few new tests to one of the test scripts in t/ is in
order, then.

^ permalink raw reply

* Re: [PATCH] Teach/Fix pull/fetch -q/-v options
From: Junio C Hamano @ 2008-10-20 23:54 UTC (permalink / raw)
  To: Tuncer Ayaz; +Cc: Junio C Hamano, git
In-Reply-To: <4ac8254d0810200935sf7ad873tea53c0fb53bbe1c0@mail.gmail.com>

"Tuncer Ayaz" <tuncer.ayaz@gmail.com> writes:

> On Sun, Oct 19, 2008 at 11:26 PM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>>> @@ -23,6 +24,10 @@ rebase=$(git config --bool branch.$curr_branch_short.rebase)
>>>  while :
>>>  do
>>>       case "$1" in
>>> +     -q|--quiet)
>>> +             verbosity="$verbosity -q" ;;
>>> +     -v|--verbose)
>>> +             verbosity="$verbosity -v" ;;
>>
>> You know verbosity flags (-q and -v) are "the last one wins", so I do not
>> see much point in this concatenation.
>
> Without concatenation I would need to analyze the content
> of the variable each time the option is passed to the shell
> script. Do you know of a simpler/better way still keeping the
> functionality that
> $ git pull -q -v --quiet --verbose --quiet gives verbosity=QUIET
> and
> $ git pull -q -v --quiet --verbose --quiet -v yields verbosity=VERBOSE
> ?

Wouldn't

	verbosity=
	while :
        do
        	case "$1" in
                -q|--quiet) verbosity=-q ;;
                -v|--verbose) verbosity=-v ;;
		... others ...
                esac
                shift
	done
        git pull $verbosity other options

give the -q for the former and -v for the latter to "git pull"?

^ permalink raw reply

* [PATCH 1/2] document "intent to add" option to git-add
From: Jeff King @ 2008-10-21  0:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This was added by 3942581 but never documented.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/git-add.txt |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 2b6d6c8..6fc20b0 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -9,8 +9,8 @@ SYNOPSIS
 --------
 [verse]
 'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
-	  [--all | [--update | -u]] [--refresh] [--ignore-errors] [--]
-	  <filepattern>...
+	  [--all | [--update | -u]] [--intent-to-add | -N]
+	  [--refresh] [--ignore-errors] [--] <filepattern>...
 
 DESCRIPTION
 -----------
@@ -92,6 +92,15 @@ OPTIONS
 	and add all untracked files that are not ignored by '.gitignore'
 	mechanism.
 
+
+-N::
+--intent-to-add::
+	Record only the fact that the path will be added later. An entry
+	for the path is placed in the index with no content. This is
+	useful for, among other things, showing the unstaged content of
+	such files with 'git diff' and commiting them with 'git commit
+	-a'.
+
 --refresh::
 	Don't add the file(s), but only refresh their stat()
 	information in the index.
-- 
1.6.0.2.770.gb4241.dirty

^ permalink raw reply related

* [PATCH 2/2] rm: loosen safety valve for empty files
From: Jeff King @ 2008-10-21  0:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

If a file is different between the working tree copy, the
index, and the HEAD, then we do not allow it to be deleted
without --force.

However, this is overly tight in the face of "git add
--intent-to-add":

  $ git add --intent-to-add file
  $ : oops, I don't actually want to stage that yet
  $ git rm --cached file
  error: 'file' has staged content different from both the
  file and the HEAD (use -f to force removal)
  $ git rm -f --cached file

This patch loosens the safety valve to allow the deletion
only if we are deleting the cached entry and the cached
content is empty.

This covers the intent-to-add situation, and presumably
there is little harm in not protecting users who have
legitimately added an empty file. In many cases, the file
will still be empty, in which case the safety valve does not
trigger anyway (since the content remains untouched in the
working tree). Otherwise, we do remove the fact that no
content was staged, but given that the content is by
definition empty, it is not terribly difficult for a user to
recreate it.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-rm.c  |    3 ++-
 cache.h       |    1 +
 read-cache.c  |    2 +-
 t/t3600-rm.sh |    7 +++++++
 4 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/builtin-rm.c b/builtin-rm.c
index e06640c..b7126e3 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -79,7 +79,8 @@ static int check_local_mod(unsigned char *head, int index_only)
 		     || hashcmp(ce->sha1, sha1))
 			staged_changes = 1;
 
-		if (local_changes && staged_changes)
+		if (local_changes && staged_changes &&
+		    !(index_only && is_empty_blob_sha1(ce->sha1)))
 			errs = error("'%s' has staged content different "
 				     "from both the file and the HEAD\n"
 				     "(use -f to force removal)", name);
diff --git a/cache.h b/cache.h
index ae6647e..7141705 100644
--- a/cache.h
+++ b/cache.h
@@ -517,6 +517,7 @@ static inline void hashclr(unsigned char *hash)
 {
 	memset(hash, 0, 20);
 }
+extern int is_empty_blob_sha1(const unsigned char *sha1);
 
 int git_mkstemp(char *path, size_t n, const char *template);
 
diff --git a/read-cache.c b/read-cache.c
index 780f2c7..d624cb3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -160,7 +160,7 @@ static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)
 	return 0;
 }
 
-static int is_empty_blob_sha1(const unsigned char *sha1)
+int is_empty_blob_sha1(const unsigned char *sha1)
 {
 	static const unsigned char empty_blob_sha1[20] = {
 		0xe6,0x9d,0xe2,0x9b,0xb2,0xd1,0xd6,0x43,0x4b,0x8b,
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 66aca99..88ae672 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -187,6 +187,13 @@ test_expect_success 'but with -f it should work.' '
 	test_must_fail git ls-files --error-unmatch baz
 '
 
+test_expect_success 'ok to remove cached empty file' '
+	touch empty &&
+	git add empty &&
+	echo content >empty &&
+	git rm --cached empty
+'
+
 test_expect_success 'Recursive test setup' '
 	mkdir -p frotz &&
 	echo qfwfq >frotz/nitfol &&
-- 
1.6.0.2.770.gb4241.dirty

^ permalink raw reply related

* Re: [PATCH] rebase-i-p: only list commits that require rewriting in todo
From: Jeff King @ 2008-10-21  0:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Stephen Haberman, git
In-Reply-To: <7vej2a3kl5.fsf@gitster.siamese.dyndns.org>

On Mon, Oct 20, 2008 at 04:36:38PM -0700, Junio C Hamano wrote:

> I do not remember the individual patches in the series, but I have to say
> that the script at the tip of the topic is, eh, less than ideal.
> 
> Here is a small untested patch to fix a few issues I spotted while reading
> it for two minutes.
> 
>  * Why filter output from "rev-list --left-right A...B" and look for the
>    ones that begin with ">"?  Wouldn't "rev-list A..B" give that?
> 
>  * The abbreviated SHA-1 are made with "rev-list --abbrev=7" into $TODO in
>    an earlier invocation, and it can be more than 7 letters to avoid
>    ambiguity.  Not just that "${r:0:7} is not even in POSIX", but use of
>    it here is actively wrong.
> 
>  * There is no point in catting a single file and piping it into grep.

All of those look like sane changes to me (I'll admit that before I
didn't even look at the script beyond the breakage on my test box).

-Peff

^ permalink raw reply

* Re: [RFC PATCH] commit: Warn about encodings unsupported by iconv.
From: Junio C Hamano @ 2008-10-21  0:39 UTC (permalink / raw)
  To: Alexander Gavrilov; +Cc: git, Paul Mackerras
In-Reply-To: <1224537918-14024-1-git-send-email-angavrilov@gmail.com>

Alexander Gavrilov <angavrilov@gmail.com> writes:

> diff --git a/builtin-commit-tree.c b/builtin-commit-tree.c
> index 0453425..7f325a3 100644
> --- a/builtin-commit-tree.c
> +++ b/builtin-commit-tree.c
> @@ -45,6 +45,28 @@ static const char commit_utf8_warn[] =
>  "You may want to amend it after fixing the message, or set the config\n"
>  "variable i18n.commitencoding to the encoding your project uses.\n";
>  
> +static const char commit_bad_encoding_warn[] =
> +"Warning: commit encoding '%s' is not supported.\n"
> +"You may want to change the value of the i18n.commitencoding config\n"
> +"variable, and redo the commit. Use 'iconv --list' to see the list of\n"
> +"available encoding names.\n";
> +
> +static void verify_commit_encoding(const char *text, const char *encoding)
> +{
> +	if (is_encoding_utf8(encoding)) {
> +		if (!is_utf8(text))
> +			fprintf(stderr, commit_utf8_warn);
> +	}
> +#ifndef NO_ICONV
> +	else {
> +		char *conv = reencode_string("", "utf-8", encoding);
> +		if (!conv)
> +			fprintf(stderr, commit_bad_encoding_warn, encoding);
> +		free(conv);
> +	}
> +#endif
> +}
> +

I think the issue you are attempting to tackle is worth addressing, but I
am not sure if this is the best approach.

Commit_tree() does not re-encode the payload.

It just marks it with the encoding header.  Wouldn't that mean that it
should be possible for you to create a commit with its message encoded in
KOI-8, and mark the resulting commit as encoded as such, on a host that is
incapable of actually transcoding from KOI-8 to utf-8?  IOW, your being
able to encode from i18n.commitencoding to utf-8 does not have much to do
with the validity of the configuration variable.

It would clarify the issues to think about what this new code would do on
a host without iconv, if you do not have the above #ifndef/#endif pair.
The replacement reencode_string() implementation always returns NULL, so
the code will always warn.

I am guessing that the reason you added #ifndef/#endif is because what the
warning message says is incorrect.

 * "is not supported" is not correct.  "is not supported HERE" may be.

 * "is not supported" (nor "is not supported HERE") does not matter.  It
   is log-reading side that does the re-encoding, not the commit
   generating side.

 * what you would really want to say is "might be incorrectly spelled",
   but your problem is that you do not have a direct way to check that.

Another reason of your "#ifndef/#endif" would be that there is no way to
squelch the warning message after seeing it on a NO_ICONV platform.

But that suggests that the "#ifndef/#endif" is not a good way to squelch
the message.  What would you do, after seeing the warning message and
examining the situation, you know KOI-8 is a valid encoding name, your
editor is recording what you type in the commit log message in KOI-8, you
know you set i18n.commitencoding to KOI-8, and you know somehow your
system is incapable of reencode_string("", "utf-8" "KOI-8")?  There is no
way to squelch the message.

So perhaps you would need some "state" variable that says "I know this
i18n.commitencoding configuration is valid" if you go this route?  But the
reason for "I know" would be either (1) because we earlier tried
reencode_string() and it resulted in an Ok return, or (2) because the user
verified that the configuration is valid, even though on this particular
system it cannot be encoded to utf-8.  IOW, the latter one would be
"because the user tells us so" --- which would be the same as trusting
i18n.commitencoding from the beginning.  I dunno.

I actually have an alternative approach to suggest.

How about adding a new i18n.commit-reencode-logmessage option (boolean),
and when it is set, we actually re-encode from i18n.commitencoding to
"utf-8" before creating the commit object (and obviously we do not store
"encoding" header in the resulting commit)?  When the conversion fails, we
know it failed, so the warning you added does make sense in that context.

^ permalink raw reply

* [PATCH] rehabilitate 'git index-pack' inside the object store
From: Nicolas Pitre @ 2008-10-21  1:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Before commit d0b92a3f6e it was possible to run 'git index-pack'
directly in the .git/objects/pack/ directory.  Restore that ability.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---

diff --git a/index-pack.c b/index-pack.c
index 0a917d7..79f6fd6 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -878,10 +877,26 @@ int main(int argc, char **argv)
 	char *index_name_buf = NULL, *keep_name_buf = NULL;
 	struct pack_idx_entry **idx_objects;
 	unsigned char pack_sha1[20];
-	int nongit = 0;
 
-	setup_git_directory_gently(&nongit);
-	git_config(git_index_pack_config, NULL);
+	/*
+	 * We wish to read the repository's config file if any, and
+	 * for that it is necessary to call setup_git_directory_gently().
+	 * However if the cwd was inside .git/objects/pack/ then we need
+	 * to go back there or all the pack name arguments will be wrong.
+	 * And in that case we cannot rely on any prefix returned by 
+	 * setup_git_directory_gently() either.
+	 */
+	{
+		char cwd[PATH_MAX+1];
+		int nongit;
+
+		if (!getcwd(cwd, sizeof(cwd)-1))
+			die("Unable to get current working directory");
+		setup_git_directory_gently(&nongit);
+		git_config(git_index_pack_config, NULL);
+		if (chdir(cwd))
+			die("Cannot come back to cwd");
+	}
 
 	for (i = 1; i < argc; i++) {
 		char *arg = argv[i];

^ permalink raw reply related

* Re: [PATCH 2/2] rm: loosen safety valve for empty files
From: Junio C Hamano @ 2008-10-21  2:50 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20081021003712.GB32569@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> This covers the intent-to-add situation, and presumably
> there is little harm in not protecting users who have
> legitimately added an empty file. In many cases, the file
> will still be empty, in which case the safety valve does not
> trigger anyway (since the content remains untouched in the
> working tree). Otherwise, we do remove the fact that no
> content was staged, but given that the content is by
> definition empty, it is not terribly difficult for a user to
> recreate it.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> ...
> +test_expect_success 'ok to remove cached empty file' '
> +	touch empty &&
> +	git add empty &&
> +	echo content >empty &&
> +	git rm --cached empty
> +'

I am actually of two minds about this patch.

With one of the commits in the nd/narrow series, we can easily add more
flag bits to the index entries, and it is conceivable that we would want
to change the "add -N" implementation to set an "intent to add" bit (which
we don't), in addition to registering an empty blob at the path (which we
currently do).  I envision that such a change would allow us to:

 - let "git diff" continue to diff with an emptiness and keep showing what
   people would expect;

 - teach "git write-tree" (and various "commit" building commands) to
   either (1) ignore a staged empty blob when the "intent to add" bit is
   set, or (2) warn and abort, saying "you told me you will tell me what
   the actual contents will be later, but you haven't done so -- I'll
   refuse to operate until you make up your mind";

in addition to what you are trying to fix here with "git rm".  With such a
change, your "git rm empty" code can also distinguish between an empty
blob the user wanted to add _as the final contents_, and a path that has
been marked with "add -N", and behave differently (the former would not
require -f while the latter would).

As an interim measure, I suspect your patch is an improvement from the
current state of affairs, but the above test will then break when an
improvement to "git add -N" implementation outlined above materializes.

So how about changing the test to explicitly check that a path that was
added by "git add -N" can be removed, and either (1) not check about an
empty blob that was explicitly added by the user, or (2) check that an
empty blob that was explicitly added by the user cannot be "git rm"'ed
without -f, with expect_failure?

^ permalink raw reply

* Re: Archiving tags/branches?
From: Pete Harlan @ 2008-10-21  2:53 UTC (permalink / raw)
  To: Johan Herland; +Cc: git, SZEDER Gábor
In-Reply-To: <200810200953.45339.johan@herland.net>

Johan Herland wrote:
> On Monday 20 October 2008, Pete Harlan wrote:
>> Johan Herland wrote:
>>> BTW, the best way IMHO to archive old refs is to clone your repo (with
>>> all tags/branches) to a backup disk, and then regularly push (git push
>>> --all && git push --tags) your new tags/branches to this backup. You
>>> are now free to delete these tags/branches from your work repo (they
>>> will not be deleted from the backup unless you use "git push
>>> --mirror"). And if you ever need to retrieve an old tag/branch, it's
>>> just a matter of pulling it from the backup repo. Nice, clean,
>>> flexible, and requires no changes to git.
>>>
>>>
>>> Have fun! :)
>>>
>>> ...Johan
>> Hi,
>>
>> Thank you; that indeed seems to work and solves the problem of managing
>> refs/archived-tags manually.
>>
>> Using a secondary repo solely to overcome a flat tag/branch namespace
>> feels hackish.  Perhaps git will benefit someday from work in this area,
>> but until I come up with a patch your suggestion should work fine.  Just
>> knowing I didn't overlook an existing feature helps a lot.
> 
> From reading your other emails, I get the feeling that I'm in a similar 
> situation at $dayjob (i.e. converting ~9 years of development history from 
> CVS to Git). We have literally tens of thousands of tags (mostly build and 
> release tags) in some of our repos, and keeping all these tags in our daily 
> work repos is simply unwieldy and impractical. We therefore plan to have 
> official reps which only contain the most important tags, and 
> have "archive" repos in a different location that contain all the other 
> tags.

Another solution that may work for me is to bind the old lines of
development together using the merge strategy "ours" to link them in a
chain.  When I first read about "ours" I thought it only has evil
applications, but it seems to be created for just this sort of tying
together development that is actually, but was not historically, linked.
 Making those tags reachable from current heads would allow stashing
them in a versioned file somewhere without cluttering up the real
tags/branches or requiring a separate repo.

> You seem to want to keep all your tags in the work repo, but in a 
> separate/hidden namespace, so that they don't clutter the default tag 
> listings. IMHO, once you get into thousands of tags, cloning and other 
> operations where all refs are synchronized become annoyingly slow (although 
> things are certainly somewhat better in v1.6). At that point, my only 
> advice is to keep the lesser-used tags in separate repos, and pull each ref 
> into your work repos on-demand, especially when most of these tags will 
> probably never be referenced.

The efficiency issue is one I hadn't considered; thanks.

--Pete

^ permalink raw reply

* Re: Archiving tags/branches?
From: Pete Harlan @ 2008-10-21  4:08 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Johan Herland, git, SZEDER Gabor
In-Reply-To: <m3prlvibb7.fsf@localhost.localdomain>

Jakub Narebski wrote:
> Pete Harlan <pgit@pcharlan.com> writes:
> 
>> Johan Herland wrote:
> 
>>> BTW, the best way IMHO to archive old refs is to clone your repo
>>> (with all tags/branches) to a backup disk, and then regularly push
>>> (git push --all && git push --tags) your new tags/branches to this
>>> backup. You are now free to delete these tags/branches from your
>>> work repo (they will not be deleted from the backup unless you use
>>> "git push --mirror"). And if you ever need to retrieve an old
>>> tag/branch, it's just a matter of pulling it from the backup
>>> repo. Nice, clean, flexible, and requires no changes to git.
>> Thank you; that indeed seems to work and solves the problem of managing
>> refs/archived-tags manually.
>>
>> Using a secondary repo solely to overcome a flat tag/branch namespace
>> feels hackish.  Perhaps git will benefit someday from work in this area,
>> but until I come up with a patch your suggestion should work fine.  Just
>> knowing I didn't overlook an existing feature helps a lot.
> 
> I don't quite understand what you mean by _flat_ namespace for tags
> and branches.
> 
> First, it is not unusual to have hierarchical branch names, at least
> for short-term topic branches. For example in git.git history (and in
> "What's cooking..." announcements on git mailing list) you can find
> branch names such as rs/alloc-ref, nd/narrow, tr/workflow-doc.
> Additionally remote-tracking branch names have inherently hierarchical
> names: refs/remotes/<remote>/<remote branch>.  While tag names usually
> are of the type x.y.z, it is not mandated by some technological
> limitation.

What I mean by "flat" is that "/" is just another character as far as
what git exposes to the user.  Regardless of any semantics the user
chooses to assign to it, and regardless of what advantage git makes use
of "/" internally, unless I can do something like:

% git tag --ls
sometag
someothertag
releases/
% git tag --ls releases/
releases/2008/
releases/2007/
% git tag --ls releases/2008
releases/2008/r3.14
%

"/" is just like any another character in a tag or branch.

(The above notional --ls modifier is probably very easy to write, and if
I do so it may address all of my woes.  Subversion's branching/tagging
can be organized pretty much exactly like this, and importing into git
such a repository is what initially led me to ask about organizing tags
and branches.)

What I'm usually likely to want from a "list tags" command is to see the
most recent few tags, not (say) all 226 tags in git.git.  I'll probably
write a little alias that does that, but even then when looking at the
whole list it would be nice to have the option to navigate it
hierarchically.  (Or in some other manner, and/or possibly with a
configurable directory separator.)

> Second, you can always put your archived refs in another namespace,
> beside 'heads', 'tags', and 'remotes'. I for example use
> refs/tags/Attic for lightweigth tags to some interesting abandoned
> experiments, but it could have been refs/deleted/tags, or
> refs/Attic/tags.

My original question was asking whether this sort of thing would work
(e.g., they would never be automatically pruned), and I'm happy to see
that the answer is yes.  The main downside to it is that you can't
clone/pull/push changes to it using git.

Many thanks to you and everyone for their help.  Git is so flexible that
it can be difficult when starting out to know whether you're missing a
way of attacking a problem.

--Pete

> Last, please remember that there exists something like packed refs
> format (see git-pack-refs(1)... oops, it dies not describe
> .git/packed-refs format, unfortunately).

^ permalink raw reply

* [PATCH] (squash) add index-pack with git-dir test
From: Junio C Hamano @ 2008-10-21  5:03 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <alpine.LFD.2.00.0810202110380.26244@xanadu.home>

This tries to make sure that the recent fix to d0b92a3 (index-pack: setup
git repository, 2008-08-26) does not introduce regression, and protects
the fix from getting broken again.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * And unfortunately the first one fails with the fix and succeeds
   without; the second one succeeds with your fix and fails without.

 t/t5302-pack-index.sh |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
index 6424db1..2edab73 100755
--- a/t/t5302-pack-index.sh
+++ b/t/t5302-pack-index.sh
@@ -177,4 +177,24 @@ test_expect_success \
        ".git/objects/pack/pack-${pack1}.pack" 2>&1) &&
      echo "$err" | grep "CRC mismatch"'
 
+test_expect_success 'running index-pack from a subdirectory' '
+    mkdir subdir &&
+    cp test-1-${pack1}.pack subdir/. &&
+    (
+	cd subdir &&
+	git index-pack test-1-${pack1}.pack
+    ) &&
+    test -f subdir/test-1-${pack1}.idx
+'
+
+test_expect_success 'running index-pack in the object store' '
+    rm -f .git/objects/pack/* &&
+    cp test-1-${pack1}.pack .git/objects/pack/pack-${pack1}.pack &&
+    (
+	cd .git/objects/pack
+	git index-pack pack-${pack1}.pack
+    ) &&
+    test -f .git/objects/pack/pack-${pack1}.idx
+'
+
 test_done
-- 
1.6.0.2.588.g31023

^ permalink raw reply related

* Re: [EGIT PATCH] git property page for project properties.
From: Robin Rosenberg @ 2008-10-21  5:37 UTC (permalink / raw)
  To: Tomi Pakarinen; +Cc: spearce, git
In-Reply-To: <1224529604-42397-1-git-send-email-tomi.pakarinen@iki.fi>

måndagen den 20 oktober 2008 21.06.44 skrev Tomi Pakarinen:
>  Show git property page in project's properties, if project
> has git repository provider.
> 
> Signed-off-by: Tomi Pakarinen <tomi.pakarinen@iki.fi>
> ---
> 
>  This is similar to CVS's property page. Current
> information on page is very limited and non editable.
> 
>  Tomi.
> 
> +	Text gitDir;
> +
> +	Text branch;
> +
> +	Text id;
> +
> +	Text state;
> +
> +	Text workDir;

Should be private. I'll squash that in and patch Egit to detect
the 1.6-way of recording repository state, which I noted it does
not handle.

-- robin

^ permalink raw reply

* Re: [PATCH, RFC] diff: add option to show context between close chunks
From: Johannes Sixt @ 2008-10-21  6:09 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git Mailing List, Davide Libenzi
In-Reply-To: <48FCC8A1.5090109@lsrfire.ath.cx>

René Scharfe schrieb:
> I have to admit my main motivation was that one line gap, where a chunk
> header hid an interesting line of context.  Showing it didn't change the
> length of the patch, so I found this to be a sad wastage.

"Wastage" is relative. For a given patch, the one line of context that was
hidden by the hunk header would be welcome by a human reader, but it is
not necessarily useful if the patch is to be applied, in particular, if it
is applied to a version of the file that has *more* than one line between
the hunk contexts. This is the reason that diff does not produce 7 lines
of context between changes in -U3 mode ("you asked for 3 lines of context,
you get 3 lines of context").

BTW, nomenclature seems to have settled at the word "hunk", not "chunk".

-- Hannes

^ permalink raw reply

* Re: [RFC PATCH] commit: Warn about encodings unsupported by iconv.
From: Alex Riesen @ 2008-10-21  6:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alexander Gavrilov, git, Paul Mackerras
In-Reply-To: <7vmygy233r.fsf@gitster.siamese.dyndns.org>

Junio C Hamano, Tue, Oct 21, 2008 02:39:36 +0200:
> I actually have an alternative approach to suggest.
> 
> How about adding a new i18n.commit-reencode-logmessage option (boolean),
> and when it is set, we actually re-encode from i18n.commitencoding to
> "utf-8" before creating the commit object (and obviously we do not store
> "encoding" header in the resulting commit)?  When the conversion fails, we
> know it failed, so the warning you added does make sense in that context.

Maybe make the option a string, and allow to choose the target
encoding (not only utf8, but anything user wishes)?

^ permalink raw reply

* Re: [PATCH, RFC] diff: add option to show context between close  chunks
From: Johannes Sixt @ 2008-10-21  6:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: René Scharfe, Git Mailing List, Davide Libenzi
In-Reply-To: <7vabcy3k9l.fsf@gitster.siamese.dyndns.org>

Junio C Hamano schrieb:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> I think it makes sense to make 1, or even 3, the default for this
>> option for all commands that create patches intended for human
>> consumption.  The patch keeps the default at 0, though.
> 
> I think defaulting to 1 would make sense, or alternatively, just
> hardcoding that behaviour without any new option.  That would give you
> more information with the same number of patch lines, iow, upside without
> any downside.

Are you sure about the "without any downside" part? The extra context line
inhibits that the patch applies cleanly to a version of the file that has
that very line modified (including a different number of lines).

-- Hannes

^ permalink raw reply

* Re: [PATCH, RFC] diff: add option to show context between close  chunks
From: Junio C Hamano @ 2008-10-21  7:12 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: René Scharfe, Git Mailing List, Davide Libenzi
In-Reply-To: <48FD781C.2000103@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> Junio C Hamano schrieb:
>> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>> 
>>> I think it makes sense to make 1, or even 3, the default for this
>>> option for all commands that create patches intended for human
>>> consumption.  The patch keeps the default at 0, though.
>> 
>> I think defaulting to 1 would make sense, or alternatively, just
>> hardcoding that behaviour without any new option.  That would give you
>> more information with the same number of patch lines, iow, upside without
>> any downside.
>
> Are you sure about the "without any downside" part? The extra context line
> inhibits that the patch applies cleanly to a version of the file that has
> that very line modified (including a different number of lines).

Yeah.  René wanted this for _human consumption_, not mechanical patch
application, so "hardcoding" literally there in the very low level of the
diff callchain is not quite right (it would affect format-patch which is
primarily for mechanical application).

I guess you could make the hardcoded value 1 for everybody else and 0 for
format-patch.

^ permalink raw reply

* Re: Feedback outside of the user survey
From: Christian Jaeger @ 2008-10-21  7:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andreas Ericsson, Garry Dolley, Richard Hartmann, git
In-Reply-To: <7viqrm3n53.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> Christian Jaeger <christian@jaeger.mine.nu> writes:
>
>   
>>> That's partially implemented, I think (google for Nguy (or something, I'm
>>> not very god with asian names),
>>>       
>> That's not enough information for me to find what you've had in
>> mind. "stump Nguy site:marc.info" doesn't yield a result with Google.
>>     
>
> I think Andreas is referring to nd/narrow topic currently parked in 'pu'.
>
> $ git log next..36aa66d^2

Thanks. This looks nice.

(It is implementing the partial, or "sparse" as Nguyễn calls it, 
checkouts. So it seems the more useful of the two ideas is basically 
already done, at least in the sense of saving space for the checkout. It 
won't 'move'/'mount' the subdirectory to the top of the working 
directory if only a subdirectory is wanted, but as I've already realized 
and written, Git may require a full working directory anyway for 
interaction with the user during merging, and a symlink can be used 
instead to 'mount' the subdirectory where the user wants it (if the OS 
supports that). Straightforward solution.)

Christian.

^ permalink raw reply

* Re: [RFC PATCH] builtin-blame: Reencode commit messages according to git-log rules.
From: Junio C Hamano @ 2008-10-21  7:34 UTC (permalink / raw)
  To: Alexander Gavrilov; +Cc: git
In-Reply-To: <1224537879-13000-1-git-send-email-angavrilov@gmail.com>

I think this makes sense.  We'd want a testcase for the new feature before
applying the patch, though.

^ 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