Git development
 help / color / mirror / Atom feed
* [PATCH] mktree: fix a memory leak in write_tree()
From: Liu Yuan @ 2011-11-11  3:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

From: Liu Yuan <tailai.ly@taobao.com>

We forget to call strbuf_release to release the buf memory.

Signed-off-by: Liu Yuan <tailai.ly@taobao.com>
---
 builtin/mktree.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/builtin/mktree.c b/builtin/mktree.c
index 098395f..4ae1c41 100644
--- a/builtin/mktree.c
+++ b/builtin/mktree.c
@@ -60,6 +60,7 @@ static void write_tree(unsigned char *sha1)
 	}
 
 	write_sha1_file(buf.buf, buf.len, tree_type, sha1);
+	strbuf_release(&buf);
 }
 
 static const char *mktree_usage[] = {
-- 
1.7.6.1

^ permalink raw reply related

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Junio C Hamano @ 2011-11-11  5:26 UTC (permalink / raw)
  To: Johan Herland
  Cc: Linus Torvalds, Ted Ts'o, Shawn Pearce, git, James Bottomley,
	Jeff Garzik, Andrew Morton, linux-ide, LKML
In-Reply-To: <CALKQrgcJmPHZG0bbgwoH6_htQODYVJtXYyHgSb_7qRKkJkb2Yw@mail.gmail.com>

Johan Herland <johan@herland.net> writes:

> Given that we need an alternative way to transfer annotations between
> repos (using auto-follow to select the relevant set of annotations, and
> then transferring only those annotations): Can we leverage existing
> functionality in "notes" where useful (e.g. using existing notes merge
> strategies to deal with colliding annotations), while at the same time
> extending the current "notes" feature with this alternative transfer
> mechanism? FWIW, I expect there are other "notes" use cases that
> would also prefer the auto-follow only-relevant transfer behavior.
>
> So, how can we use "notes" to better support the transfer semantics you
> suggest? The mapping from the object being annotated to the annotation
> object is already contained in the notes tree, but the "timestamp" you
> describe (needed to efficiently calculate the set of annotations to
> auto-follow) is not [1].

Please do not take the "timestamp" part too seriously.

I am starting to think that what we want in this context actually is very
close to annotated tags. I said we want a mapping from an annotated object
to "a set of other objects" that annotate it, but it was an unnecessary
and premature generalization. There is no reason that these annotations
have to be structured "Git" objects such as blobs and trees.

A set of annotated tags that have the same value on their "object" field
is a perfect match for "a set of annotations attached to a given object".

We already know that using the real tags has its own problems coming from
having to give each and every one of them unique names somewhere in the
refs hierarchy (be it refs/tags/ or refs/audit/), but imagine if we
somehow had a way to:

 - keep these annotated tags in the object store;

 - keep them from getting pruned even if they are not referenced from
   anywhere in refs/ hierarchy;

 - given an object, efficiently enumerate such annotate tags that refer to
   the object.

And then imagine that we are pushing history leading to a commit from one
repository to another. Both repositories store these "anonymous" (that is
what they are---they do not have a name in the refs/ hierarchy) tags.

The two repositories can individually enumerate all these "anonymous" tags
that annotate commits in the history that is being exchanged, and run a
set reconciliation algorithm (e.g. [*1*]) to find out the anonymous tags
that are missing from the recipient repository.

Such an approach does not require any timestamp.

My point is _not_ that the alternative in this message is superiour to the
handwaving in my other message, but is that I think it may not be the best
approach to think what needs to be added to "notes" to make it applicable
for the problem we are solving.

Rather, I think we should design how the overall system should look like
(i.e. what property the resulting system should have) and then find out
what is necessary in each part of the resulting solution (i.e. the list of
"somehow had a way to..." above, plus "efficient set reconciliation").


[Footnote]

*1* What's the Difference? Efficient Set Reconciliation without Prior
Context http://cseweb.ucsd.edu/~fuyeda/papers/sigcomm2011.pdf

^ permalink raw reply

* Update Linux kernel branch source from GIT
From: Eric Kom @ 2011-11-11  5:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsjlyw0y4.fsf@alter.siamese.dyndns.org>


[-- Attachment #1.1: Type: text/plain, Size: 652 bytes --]

Good day,

Am new on this list, and also compile the kernel linux from git after
clone. since I don't use the tar kernel version, am use to clone a new
kernel branch instead to update it via patch.

Please can you explain to me how to make a patch after clone it?

This is my clone command:
 git clone
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git linux-3.2


-- 
Your Truly

Eric Kom

2 Hennie Van Till, White River, 1240
erickom@kom.za.net | erickom@namekom.co.za | erickom@erickom.co.za
www.kom.za.net | www.kom.za.org | www.erickom.co.za

Key fingerprint: 513E E91A C243 3020 8735 09BB 2DBC 5AD7 A9DA 1EF5

[-- Attachment #1.2: 0xA9DA1EF5.asc --]
[-- Type: application/pgp-keys, Size: 2211 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* [PATCH/RFC] Restore line limit option in post-receive-email
From: Cheng Leong @ 2011-11-11  6:35 UTC (permalink / raw)
  To: git; +Cc: kpfleming, Cheng Leong

The hooks.emailmaxlines config currently has no effect. Stop
prep_for_email from clobbering the already-initialized maxlines
variable in the contrib/hooks/post-receive-email example.

Signed-off-by: Cheng Leong <leongc@alumni.rice.edu>
---
 contrib/hooks/post-receive-email |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index ba077c1..ac2e0ed 100755
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -85,7 +85,6 @@ prep_for_email()
 	oldrev=$(git rev-parse $1)
 	newrev=$(git rev-parse $2)
 	refname="$3"
-	maxlines=$4
 
 	# --- Interpret
 	# 0000->1234 (create)
-- 
1.7.7.1.msysgit.0

^ permalink raw reply related

* Re: [PATCH/RFC] Restore line limit option in post-receive-email
From: Junio C Hamano @ 2011-11-11  6:56 UTC (permalink / raw)
  To: Cheng Leong; +Cc: git, kpfleming
In-Reply-To: <1320993311-27112-1-git-send-email-leongc@alumni.rice.edu>

Cheng Leong <leongc@alumni.rice.edu> writes:

> The hooks.emailmaxlines config currently has no effect. Stop
> prep_for_email from clobbering the already-initialized maxlines
> variable in the contrib/hooks/post-receive-email example.
>
> Signed-off-by: Cheng Leong <leongc@alumni.rice.edu>
> ---
>  contrib/hooks/post-receive-email |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
> index ba077c1..ac2e0ed 100755
> --- a/contrib/hooks/post-receive-email
> +++ b/contrib/hooks/post-receive-email
> @@ -85,7 +85,6 @@ prep_for_email()
>  	oldrev=$(git rev-parse $1)
>  	newrev=$(git rev-parse $2)
>  	refname="$3"
> -	maxlines=$4
>  
>  	# --- Interpret
>  	# 0000->1234 (create)

Umm, there is another place where $maxlines is used without
merit. Shouldn't we do something like below as well?

 contrib/hooks/post-receive-email |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index ba077c1..e27ca3c 100755
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -743,6 +743,6 @@ else
 	while read oldrev newrev refname
 	do
 		prep_for_email $oldrev $newrev $refname || continue
-		generate_email $maxlines | send_mail
+		generate_email | send_mail
 	done
 fi

^ permalink raw reply related

* Re: Disappearing change on pull rebase
From: Johannes Sixt @ 2011-11-11  6:56 UTC (permalink / raw)
  To: Pitucha, Stanislaw Izaak; +Cc: git@vger.kernel.org
In-Reply-To: <3FF1328CB05DB74898F769F1BA17812C3E49B74699@GVW1348EXA.americas.hpqcorp.net>

Am 11/10/2011 14:35, schrieb Pitucha, Stanislaw Izaak:
> As mentioned in the original mail - the merge commit did have changes.
> Here's the log of reproducing it. The line containing "2" in changelog
> is gone from master after pull --rebase.
> ...
> disappearing_commit$ git merge --no-ff --no-commit some-branch
> Automatic merge went well; stopped before committing as requested
> disappearing_commit$ echo 2 >> changelog 
> disappearing_commit$ git add changelog
> disappearing_commit$ git commit
> [master e41e4c9] Merge branch 'some-branch'

This is by design. Rebase does not rebase merge commits because it is
assumed that merge commits only do what their name implies - to merge
branches of a forked history. As such, they do not introduce their own
changes. Follow this rule, i.e., make your change in a separate non-merge
commit, and you are fine.

-- Hannes

^ permalink raw reply

* Re: [PATCH/RFC] Restore line limit option in post-receive-email
From: Cheng Leong @ 2011-11-11  7:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, kpfleming
In-Reply-To: <7v4nybrvug.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> Umm, there is another place where $maxlines is used without
> merit. Shouldn't we do something like below as well?
...
> -               generate_email $maxlines | send_mail
> +               generate_email | send_mail

Agree. $maxlines is harmless, but extraneous here.
Would you like me to reroll a patch with both or is this a trivial fixup?

Cheng

^ permalink raw reply

* Re: Disappearing change on pull rebase
From: Philippe Vaucher @ 2011-11-11  9:50 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Pitucha, Stanislaw Izaak, git@vger.kernel.org
In-Reply-To: <4EBCC71D.6000505@viscovery.net>

> This is by design. Rebase does not rebase merge commits because it is
> assumed that merge commits only do what their name implies - to merge
> branches of a forked history. As such, they do not introduce their own
> changes. Follow this rule, i.e., make your change in a separate non-merge
> commit, and you are fine.

Doesn't this create a problem if you pull, resolve a conflit but do NOT
push, then pull --rebase some more commits later on? As I understand
it, the conflict resolution commit will be a merge commit and will be
thrown away by the git pull --rebase.

Philippe

^ permalink raw reply

* Re: Disappearing change on pull rebase
From: Johannes Sixt @ 2011-11-11 10:04 UTC (permalink / raw)
  To: Philippe Vaucher; +Cc: Pitucha, Stanislaw Izaak, git@vger.kernel.org
In-Reply-To: <CAGK7Mr6D6-4aNceTDCYTHabA3vnxh+uvQ=GOeS_3nrL9rjmc9w@mail.gmail.com>

Am 11/11/2011 10:50, schrieb Philippe Vaucher:
>> This is by design. Rebase does not rebase merge commits because it is
>> assumed that merge commits only do what their name implies - to merge
>> branches of a forked history. As such, they do not introduce their own
>> changes. Follow this rule, i.e., make your change in a separate non-merge
>> commit, and you are fine.
> 
> Doesn't this create a problem if you pull, resolve a conflit but do NOT
> push, then pull --rebase some more commits later on? As I understand
> it, the conflict resolution commit will be a merge commit and will be
> thrown away by the git pull --rebase.

You are correct, but it is not a problem: During the rebase, the same
conflicts will arise as during the merge, and you will be forced to
resolve them before you can complete the rebase. Therefore, nothing will
be lost.

-- Hannes

^ permalink raw reply

* Re: Disappearing change on pull rebase
From: Philippe Vaucher @ 2011-11-11 10:08 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Pitucha, Stanislaw Izaak, git@vger.kernel.org
In-Reply-To: <4EBCF34A.3090908@viscovery.net>

>> Doesn't this create a problem if you pull, resolve a conflit but do NOT
>> push, then pull --rebase some more commits later on? As I understand
>> it, the conflict resolution commit will be a merge commit and will be
>> thrown away by the git pull --rebase.
>
> You are correct, but it is not a problem: During the rebase, the same
> conflicts will arise as during the merge, and you will be forced to
> resolve them before you can complete the rebase. Therefore, nothing will
> be lost.

Doing the same conflict resolution twice is kinda irritating... but
ok, fair enough.
I guess when you resolve a conflict you're supposed to push to avoid this :)

Philippe

^ permalink raw reply

* Re: [RFC/PATCH] i18n: add infrastructure for translating Git with gettext
From: Jakub Narebski @ 2011-11-11 10:27 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git
In-Reply-To: <1320970164-31694-2-git-send-email-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> = Perl
> 
> Perl code that wants to be localized should use the new Git::I18n
> module. It imports a __ function into the caller's package by
> default.
> 
> Instead of using the high level Locale::TextDomain interface I've
> opted to use the low-level (equivalent to the C interface)
> Locale::Messages module, which Locale::TextDomain itself uses.

Nice of you using libintl-perl instead of old Locale::MakeText
(with gettext compatibility layer).

> See <AANLkTilYD_NyIZMyj9dHtVk-ylVBfvyxpCC7982LWnVd@mail.gmail.com> for
> a further elaboration on this topic.

http://thread.gmane.org/gmane.comp.version-control.git/148446/focus=148478

> = Shell
> 
> Shell code that's to be localized should use the git-sh-i18n
> library. It's basically just a wrapper for the system's gettext.sh.
> 
> If gettext.sh isn't available we'll fall back on gettext(1) if it's
> available. The latter is available without the former on Solaris,
> which has its own non-GNU gettext implementation. We also need to
> emulate eval_gettext() there.
>
> If neither are present we'll use a dumb printf(1) fall-through
> wrapper.
> 
> I originally tried to detect if the system supported `echo -n' but
> I found this to be a waste of time. My benchmarks on Linux, Solaris
> and FreeBSD reveal that printf(1) is fast enough, especially since
> we aren't calling gettext() from within any tight loops, and
> unlikely to ever do so.

Didn't we decide that the only sane way to handle eval_gettext
is to provide minimal implemetation in the form of external command?
 

Thanks for working on this.
-- 
Jakub Narębski

^ permalink raw reply

* Re: [RFC/PATCH] i18n: add infrastructure for translating Git with gettext
From: Ævar Arnfjörð Bjarmason @ 2011-11-11 10:34 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <m31utfnedi.fsf@localhost.localdomain>

On Fri, Nov 11, 2011 at 11:27, Jakub Narebski <jnareb@gmail.com> wrote:

>> I originally tried to detect if the system supported `echo -n' but
>> I found this to be a waste of time. My benchmarks on Linux, Solaris
>> and FreeBSD reveal that printf(1) is fast enough, especially since
>> we aren't calling gettext() from within any tight loops, and
>> unlikely to ever do so.
>
> Didn't we decide that the only sane way to handle eval_gettext
> is to provide minimal implemetation in the form of external command?

This comment is a bit out of date and probably shouldn't be in the
commit message. But it doesn't have to do with eval_gettext. I mean at
one point I tried to change the fall-through wrapper from:

    gettext () {
	    printf "%s" "$1"
	}

to:

    gettext () {
	    echo -n $1
	}

But found it to bee too troublesome with various echo implementations
and just not worth it for the complexity.

But yeah, for eval_gettext() we need an external program, which we
already have in git.git as git-sh-i18n--envsubst.

^ permalink raw reply

* Re: [RFC/PATCH] remote: add new sync command
From: Jakub Narebski @ 2011-11-11 10:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Felipe Contreras, Jeff King, git
In-Reply-To: <7vobwmvuei.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > Perhaps these 'git remote' commands should be removed in 1.8 then.
> 
> It is true that it was a long-term goal to deprecate many parts of the
> "git remote" script that started as a hack to scratch itches "git fetch"
> in the older days did not directly scratch for people, e.g. fetching from
> multiple remotes in one go.
> 
> I do not think 1.7.X series to 1.8 is a big enough jump to remove
> duplicated features, though.
 
I am using "git remote update" to fetch a _subset_ of remotes;
does "git fetch" offers such feature already?

-- 
Jakub Narębski

^ permalink raw reply

* Re: [RFC/PATCH] remote: add new sync command
From: Felipe Contreras @ 2011-11-11 12:30 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20111108181442.GA17317@sigill.intra.peff.net>

On Tue, Nov 8, 2011 at 8:14 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 08, 2011 at 07:31:09PM +0200, Felipe Contreras wrote:
>
>> >  1. git push --prune <remote> :
>> >
>> >     I.e., use the "matching" refspec to not push new things, but turn
>> >     on pruning.
>>
>> I guess so, but ":" seems a bit obscure.
>
> Yeah, it is. It's also the default, so you could just do:
>
>  git push --prune <remote>

That would work only if not configured otherwise; remote.<foo>.push,
push.default

> Although some people have argued for changing the default in future
> versions. I don't know what the status of that is.

IMO the default doesn't matter, it should be easy for everyone.

>> >  2. git push <remote> refs/heads/*
>> >
>> >     Turn off pruning, but use an explicit refspec, not just "matching",
>> >     which will push all local branches.
>>
>> Isn't refs/heads/* the same as --all? BTW. I think --all is confusing,
>> should be --branches, or something.
>
> Doesn't --all mean all refs, including tags and branches?

Nope, only branches, try it. I also found it strange. And what is
more, you can't use --all and --tags at the same time. Totally
strange.

Also, --all doesn't push other refs (say refs/foobar/test)

I think this area has been neglected.

> I thought that was the thing you were avoiding.

--all + --tags is not the same as --mirror (refs/foobar/* is pushed
only by --mirror).

And yes, in this particular use-case that's what I am trying to avoid,
but in other use-cases (like creating a new repo and pushing
*everything*), a *true* --all would be nice.

> We could add syntactic sugar to make
> --branches mean "refs/heads/*". But I do worry that pseudo-options like
> that just end up creating more confusion (I seem to recall there being a
> lot of confusion about "--tags", which is more or less the same thing).

But it's not, that could explain part of the confusion. I think these
would be totally intuitive.

 --branches
 --tags
 --other
 --all
 --update
 --prune

But what about 'git fetch'? You didn't comment anything. I think we
should try to be consistent in these imaginary future options, maybe
to devise a transition, or at least to identify good names for the new
options.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* Re: Thoughts on gitk's memory footprint over linux-2.6.git
From: Felipe Contreras @ 2011-11-11 12:44 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: Git Mailing List
In-Reply-To: <CACPiFC+T1EZ1CSakQxsYZhsnHc-ZsN1-=tpoi-NaQSdpU5Yxkg@mail.gmail.com>

On Mon, Sep 26, 2011 at 10:38 PM, Martin Langhoff
<martin.langhoff@gmail.com> wrote:
> However, I find it extremely annoying over the kernel tree, due to its
> memory footprint. It is not the only thing I am running, (Chrome
> Browser, Gnome3, Firefox, many gnome Terminal windows, emacs), and
> given that I am looking at "just a couple of commits" I don't feel
> opening a few gitk instances should be problematic... except that it
> is.

Sometimes I do this:
 % gitk master..branch_1 master..branch_2 ...

But as I visualize more branches, it becomes tedious.

It would be nice to have --base option, and show only the commits
<base>..<branch>.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply

* feature request: git annotate -w like git blame -w
From: Raoul Bhatia [IPAX] @ 2011-11-11 13:02 UTC (permalink / raw)
  To: git

hi!

is it possible to add a "git annotate -w" option like git blame has?

thanks,
raoul
ps. please reply to me in cc as i'm not subscribed to this list.
-- 
____________________________________________________________________
DI (FH) Raoul Bhatia M.Sc.          email.          r.bhatia@ipax.at
Technischer Leiter

IPAX - Aloy Bhatia Hava OG          web.          http://www.ipax.at
Barawitzkagasse 10/2/2/11           email.            office@ipax.at
1190 Wien                           tel.               +43 1 3670030
FN 277995t HG Wien                  fax.            +43 1 3670030 15
____________________________________________________________________

^ permalink raw reply

* Re: [PATCH 0/14] resumable network bundles
From: David Michael Barr @ 2011-11-11 13:13 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20111110074330.GA27925@sigill.intra.peff.net>

On Thu, Nov 10, 2011 at 6:43 PM, Jeff King <peff@peff.net> wrote:
> One possible option for resumable clones that has been discussed is
> letting the server point the client by http to a static bundle
> containing most of history, followed by a fetch from the actual git repo
> (which should be much cheaper now that we have all of the bundled
> history).  This series implements "step 0" of this plan: just letting
> bundles be fetched across the network in the first place.
>
> Shawn raised some issues about using bundles for this (as opposed to
> accessing the packfiles themselves); specifically, this raises the I/O
> footprint of a repository that has to serve both the bundled version of
> the pack and the regular packfile.
>
> So it may be that we don't follow this plan all the way through.
> However, even if we don't, fetching bundles over http is still a useful
> thing to be able to do. Which makes this first step worth doing either
> way.
>
>  [01/14]: t/lib-httpd: check for NO_CURL
>  [02/14]: http: turn off curl signals
>  [03/14]: http: refactor http_request function
>  [04/14]: http: add a public function for arbitrary-callback request
>  [05/14]: remote-curl: use http callback for requesting refs
>  [06/14]: transport: factor out bundle to ref list conversion
>  [07/14]: bundle: add is_bundle_buf helper
>  [08/14]: remote-curl: free "discovery" object
>  [09/14]: remote-curl: auto-detect bundles when fetching refs
>  [10/14]: remote-curl: try base $URL after $URL/info/refs
>  [11/14]: progress: allow pure-throughput progress meters
>  [12/14]: remote-curl: show progress for bundle downloads
>  [13/14]: remote-curl: resume interrupted bundle transfers
>  [14/14]: clone: give advice on how to resume a failed clone
>
> -Peff
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

I just want to say thank you for doing this.--David Barr

^ permalink raw reply

* Re: feature request: git annotate -w like git blame -w
From: Jakub Narebski @ 2011-11-11 13:57 UTC (permalink / raw)
  To: Raoul Bhatia [IPAX]; +Cc: git
In-Reply-To: <4EBD1CF4.7040002@ipax.at>

"Raoul Bhatia [IPAX]" <r.bhatia@ipax.at> writes:

> is it possible to add a "git annotate -w" option like git blame has?

Why not use "git blame -c -w"?  `-c' turns on annotate-compatibile
output.

From git-annotate(1) manpage:

   The only difference between this command and git-blame(1) is that they  use
   slightly  different  output formats, and this command exists only for back-
   ward compatibility to support existing scripts, and provide a more familiar
   command name for people coming from other SCM systems.
 
> ps. please reply to me in cc as i'm not subscribed to this list.

This is usual behavior on this list.

-- 
Jakub Narębski

^ permalink raw reply

* Re: reset reports file as modified when it's in fact deleted
From: Carlos Martín Nieto @ 2011-11-11 14:08 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20111107162642.GA27055@sigill.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 5184 bytes --]

On Mon, Nov 07, 2011 at 11:26:42AM -0500, Jeff King wrote:
> On Mon, Nov 07, 2011 at 10:43:30AM +0100, Carlos Martín Nieto wrote:
> 
> > When I delete a file (git rm) and then reset so it exists in the index
> > again, the message tells me 'M file.txt' even though the file doesn't
> > exist in the worktree anymore. Running git status afterwards does give
> > the correct output. So, here's the minimal steps to reproduce:
> > 
> >     $ git init
> >     Initialized empty Git repository in /home/carlos/test/reset-err/.git/
> >     $ touch file.txt
> >     $ git add file.txt
> >     $ git ci file.txt -m initial
> >     [master (root-commit) a536393] initial
> >      0 files changed, 0 insertions(+), 0 deletions(-)
> >      create mode 100644 file.txt
> >     $ git rm file.txt
> >     rm 'file.txt'
> >     $ git reset -- file.txt
> >     Unstaged changes after reset:
> >     M		 file.txt
> >     $ git status -b -s
> >     ## master
> >      D file.txt
> 
> You can simplify this even further by not touching the index at all:
> 
>   git init -q &&
>   >file && git add file && git commit -q -m initial &&
>   rm file &&
>   git reset
> 
> produces:
> 
>   Unstaged changes after reset:
>   M       file

Ah, I see. I got the previous sequence because that's what we have in
an instruction manual and where we saw it.

> 
> > I'd expect the output after "Unstaged changes after reset" to tell me
> > file.txt has been deleted instead of modified. This happens with
> > 1.7.8-rc0, 1.7.7 and 1.7.4.1 and I expect with many more that I don't
> > have here.
> > 
> > I thought the index diff code might have been checking the index at the
> > wrong time, but I can run 'git reset HEAD -- file.txt' as many times
> > as I want, and it will still say 'M', so now I'm not sure.
> 
> The index diff code isn't running at all. Those messages are produced by
> refresh_index, which outputs only two flags: modified or unmerged. I
> think the reason for this is somewhat historical. You would run
> "update-index --refresh", and it would helpfully say "by the way, when
> refreshing this entry, I noticed that it is in need of being updated in
> the index". The text was "file.txt: needs update".
> 
> Later, many porcelain commands started to refresh the index themselves,
> and the "needs update" message was very confusing. So it was switched to
> the more familiar "M file.txt" (though you can still see the original
> plumbing message if you run update-index yourself).
> 
> I think it is a little more friendly to distinguish deletion from just
> modification. And there's shouldn't be any compatibility questions, as
> these are explicitly porcelain output (plumbing will still say "needs
> update").
> 
> There are a few other cases users might expect to see. We'll never show
> copies or renames, of course, because we aren't actually doing a diff
> with copy detection. We won't see an "A"dded file, because such a file
> would be in the working tree but not the index, meaning it is not
> tracked.
> 
> We could see a "T"ypechange, but the distinction between that and a
> modified file is lost by the time we get to refresh_index. We could pass
> it up, but I wonder if it's really worth it.

That's probably overkill. The issue with reporting 'M' for a deleted
file is that it conflicts with what the status output would say, even
though it's in the same format.

> 
> The patch to do "D"eleted is pretty simple:
> 
> diff --git a/read-cache.c b/read-cache.c
> index dea7cd8..cc1ebdf 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1103,9 +1103,11 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
>  	int in_porcelain = (flags & REFRESH_IN_PORCELAIN);
>  	unsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;
>  	const char *needs_update_fmt;
> +	const char *needs_rm_fmt;
>  	const char *needs_merge_fmt;
>  
>  	needs_update_fmt = (in_porcelain ? "M\t%s\n" : "%s: needs update\n");
> +	needs_rm_fmt = (in_porcelain ? "D\t%s\n" : "%s: needs update\n");
>  	needs_merge_fmt = (in_porcelain ? "U\t%s\n" : "%s: needs merge\n");

While the name fits in with the rest of the variables, it's kind of
the wrong way around, isn't it? It doesn't need an 'rm', it /was/
rm'd. Other than that stupid thing, the patch works great, thanks. Can
we get it into git?

>  	for (i = 0; i < istate->cache_nr; i++) {
>  		struct cache_entry *ce, *new;
> @@ -1145,7 +1147,10 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
>  			}
>  			if (quiet)
>  				continue;
> -			show_file(needs_update_fmt, ce->name, in_porcelain, &first, header_msg);
> +			if (cache_errno == ENOENT)
> +				show_file(needs_rm_fmt, ce->name, in_porcelain, &first, header_msg);
> +			else
> +				show_file(needs_update_fmt, ce->name, in_porcelain, &first, header_msg);
>  			has_errors = 1;
>  			continue;
>  		}
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: Thoughts on gitk's memory footprint over linux-2.6.git
From: Johannes Sixt @ 2011-11-11 14:19 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Martin Langhoff, Git Mailing List
In-Reply-To: <CAMP44s1cZc5OZ0L0zG-Wu+QVpu7xv4-JtWTBtPvnjO7sUFeM9w@mail.gmail.com>

Am 11/11/2011 13:44, schrieb Felipe Contreras:
> On Mon, Sep 26, 2011 at 10:38 PM, Martin Langhoff
> <martin.langhoff@gmail.com> wrote:
>> However, I find it extremely annoying over the kernel tree, due to its
>> memory footprint. It is not the only thing I am running, (Chrome
>> Browser, Gnome3, Firefox, many gnome Terminal windows, emacs), and
>> given that I am looking at "just a couple of commits" I don't feel
>> opening a few gitk instances should be problematic... except that it
>> is.
> 
> Sometimes I do this:
>  % gitk master..branch_1 master..branch_2 ...
> 
> But as I visualize more branches, it becomes tedious.
> 
> It would be nice to have --base option, and show only the commits
> <base>..<branch>.

What's wrong with

     gitk master..branch_1 branch_2 branch_3 branch_4 branch_5
or
     gitk --branches --not master

? (I do that all the time.) Recall that 'master..branch_1' is short for
'^master branch_1'. It is sufficient to specify the negative ref, ^master,
only once.

-- Hannes

^ permalink raw reply

* Re: Thoughts on gitk's memory footprint over linux-2.6.git
From: Felipe Contreras @ 2011-11-11 16:04 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Martin Langhoff, Git Mailing List
In-Reply-To: <4EBD2EFF.1010000@viscovery.net>

On Fri, Nov 11, 2011 at 4:19 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 11/11/2011 13:44, schrieb Felipe Contreras:
>> It would be nice to have --base option, and show only the commits
>> <base>..<branch>.
>
> What's wrong with
>
>     gitk master..branch_1 branch_2 branch_3 branch_4 branch_5
> or
>     gitk --branches --not master
>
> ? (I do that all the time.) Recall that 'master..branch_1' is short for
> '^master branch_1'. It is sufficient to specify the negative ref, ^master,
> only once.

Ah, nice! Thanks a lot :)

-- 
Felipe Contreras

^ permalink raw reply

* Re: Disappearing change on pull rebase
From: Junio C Hamano @ 2011-11-11 16:31 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Pitucha, Stanislaw Izaak, git@vger.kernel.org
In-Reply-To: <4EBCC71D.6000505@viscovery.net>

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

> Am 11/10/2011 14:35, schrieb Pitucha, Stanislaw Izaak:
>> As mentioned in the original mail - the merge commit did have changes.
>> Here's the log of reproducing it. The line containing "2" in changelog
>> is gone from master after pull --rebase.
>> ...
>> disappearing_commit$ git merge --no-ff --no-commit some-branch
>> Automatic merge went well; stopped before committing as requested
>> disappearing_commit$ echo 2 >> changelog 
>> disappearing_commit$ git add changelog
>> disappearing_commit$ git commit
>> [master e41e4c9] Merge branch 'some-branch'
>
> This is by design. Rebase does not rebase merge commits because it is
> assumed that merge commits only do what their name implies - to merge
> branches of a forked history. As such, they do not introduce their own
> changes. Follow this rule, i.e., make your change in a separate non-merge
> commit, and you are fine.

While that may be technically correct, I wonder if we can be a bit more
helpful to the users in such a case (upfront I admit that I have a strong
suspicion that this is a hard problem in general). One typical use of
"rebase" is to linearize a history that has unfortunate merges that did
not have to be there, so refusing "git rebase A..B" when there is a merge
in "git rev-list --merges A..B" is not a solution. But would it help if we
warned about the merges when we find that there is something _interesting_
going on in them, e.g. an evil merge that adds material that did not exist
in any of the parents [*1*]? The warning message may diagnose "you asked
me to linearize the history by picking commits on the non-merge segments
and replaying them, but there may be changes made in this merge commit,
and it does this interesting thing: $(git show -c $that_merge_commit)" and
may further suggest "if you do not want to linearize but just transplant
the history, perhaps you want to run the command with the '-m' option?".

[Footnote]

*1* This is a hard problem and not just the matter of looking at "show -c"
output. A "-s ours" merge would appear empty in "show -c" but it _is_ an
interesting event that makes the result of linearizing non-merge segments
vastly different from the original. Also material that did not exist in
any of the parents is not necessarily evil (e.g. the side branch may have
added one parameter to a function and updated its call sites, while our
branch may have added a different parameter to the same function. The
update to the call sites in the merge result should pass two more parameters
from the common ancestor, and different from either of the parents).

^ permalink raw reply

* Re: [RFC/PATCH] remote: add new sync command
From: Junio C Hamano @ 2011-11-11 16:38 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Felipe Contreras, Jeff King, git
In-Reply-To: <m3wrb7lzg8.fsf@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>> 
>> > Perhaps these 'git remote' commands should be removed in 1.8 then.
>> 
>> It is true that it was a long-term goal to deprecate many parts of the
>> "git remote" script that started as a hack to scratch itches "git fetch"
>> in the older days did not directly scratch for people, e.g. fetching from
>> multiple remotes in one go.
>> 
>> I do not think 1.7.X series to 1.8 is a big enough jump to remove
>> duplicated features, though.
>  
> I am using "git remote update" to fetch a _subset_ of remotes;
> does "git fetch" offers such feature already?

Heh, look at builtin/remote.c::update() and report what you see.  It just
calls into "git fetch" and let the command fetch either from a single
repository or from a remote group. "git remote update" is not even aware
of the remote groups; the expansion is done by "git fetch".

Whoever added "multiple repositories" feature to "git fetch" in order to
support "remote update <group>" apparently under-documented it.

^ permalink raw reply

* Re: reset reports file as modified when it's in fact deleted
From: Junio C Hamano @ 2011-11-11 16:43 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: Jeff King, git
In-Reply-To: <20111111140834.GA10025@beez.lab.cmartin.tk>

Carlos Martín Nieto <cmn@elego.de> writes:

> On Mon, Nov 07, 2011 at 11:26:42AM -0500, Jeff King wrote:
> ...
>> The patch to do "D"eleted is pretty simple:
>> 
>> diff --git a/read-cache.c b/read-cache.c
>> index dea7cd8..cc1ebdf 100644
>> --- a/read-cache.c
>> +++ b/read-cache.c
>> @@ -1103,9 +1103,11 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
>>  	int in_porcelain = (flags & REFRESH_IN_PORCELAIN);
>>  	unsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;
>>  	const char *needs_update_fmt;
>> +	const char *needs_rm_fmt;
>>  	const char *needs_merge_fmt;
>>  
>>  	needs_update_fmt = (in_porcelain ? "M\t%s\n" : "%s: needs update\n");
>> +	needs_rm_fmt = (in_porcelain ? "D\t%s\n" : "%s: needs update\n");
>>  	needs_merge_fmt = (in_porcelain ? "U\t%s\n" : "%s: needs merge\n");
>
> While the name fits in with the rest of the variables, it's kind of
> the wrong way around, isn't it? It doesn't need an 'rm', it /was/
> rm'd.

The variable names were chosen to mean "In a situation where the plumbing
traditionally would have said X, use this format to describe it". This is
the first topic to separate a single situation (from the plumbing's point
of view) into two and say different things at Porcelain, and the variable
naming no longer works.

An obvious solution would be to rename all of them to be based on "what
happened to the path". E.g. "modified_fmt" would be set to either "M" or
"needs update", and "removed_fmt" would be set to either "D" or "needs
update", etc.

^ permalink raw reply

* Re: [RFC/PATCH] remote: add new sync command
From: Jeff King @ 2011-11-11 18:13 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git
In-Reply-To: <CAMP44s2RjcFtdO2jft0Hg9RtqK-DRK47gX8By-dBFSBcSA+yFA@mail.gmail.com>

On Fri, Nov 11, 2011 at 02:30:48PM +0200, Felipe Contreras wrote:

> > Doesn't --all mean all refs, including tags and branches?
> 
> Nope, only branches, try it. I also found it strange. And what is
> more, you can't use --all and --tags at the same time. Totally
> strange.

Yeah, you're right. Sorry for my confusion.

So in that sense, it is poorly named, and "--branches" (or "--heads")
would be more accurate. At the same time, it is probably more likely
what the user wants to do (you almost never want to push "refs/remotes",
for example). So I am a little hesitant to suggest changing it, even
with a warning and deprecation period.

> And yes, in this particular use-case that's what I am trying to avoid,
> but in other use-cases (like creating a new repo and pushing
> *everything*), a *true* --all would be nice.

Right. It looks like that is just spelled "--mirror" (which gives you
pruning also), or "refs/*:refs/*" (without pruning). The latter is even
more flexible, as you could do "refs/*:refs/foo/*" to keep several
related backups in one upstream repo.

Just to get back to the original patch for a second: are we in agreement
that what you want to do with "sync" is almost possible now (modulo a
separate --prune option), and that there is a separate issue of making
friendlier syntax for a few common refspecs?

> > We could add syntactic sugar to make
> > --branches mean "refs/heads/*". But I do worry that pseudo-options like
> > that just end up creating more confusion (I seem to recall there being a
> > lot of confusion about "--tags", which is more or less the same thing).
> 
> But it's not, that could explain part of the confusion. I think these
> would be totally intuitive.
> 
>  --branches
>  --tags
>  --other
>  --all
>  --update
>  --prune

My problem with them syntactically is that you have option-looking
things that are really not flags, but rather syntactic placeholders for
refspecs. So you have:

  git push --prune remote --branches

which looks wrong from the perspective of usual option-parsing rules.
And does:

  git push remote --prune --branches

work? Or:

  git push --prune --branches remote

?

I think we could make them all work if we want. But somehow the syntaxes
just look wrong to me. Using a non-dashed placeholder for special
refspecs makes more sense to me like:

  git push --prune remote BRANCHES

and then it really is just a special way of spelling "refs/heads/*". But
then, I also think it's good for users to understand that the options
are refspecs, and what that means. It's not a hard concept, and then
when they inevitably say "how can I do BRANCHES, except put the result
somewhere else in the remote namespace", it's a very natural extension
to learn about the right-hand side o the refspec.

Of course I also think BRANCHES looks ugly, and people should just learn
"refs/heads/*".

I dunno. Maybe my brain is fried from knowing too much about how git
works. I don't have much of an "ordinary user" perspective anymore.

> But what about 'git fetch'? You didn't comment anything. I think we
> should try to be consistent in these imaginary future options, maybe
> to devise a transition, or at least to identify good names for the new
> options.

Yeah, fetch makes it harder because the options may have subtly
different meanings. Using non-option placeholders would work around
that. You would still have options for pruning, which to me is not
really a refspec, but an option that acts on the refspecs.

>From the list in your previous email, you wrote:

> --prune -> rename to --prune-tracking?
> --prune-local (new; prune local branches that don't exist on the remote)

I think --prune wouldn't need renaming. If you fetch into tracking
branches, then pruning would prune tracking branches. If you fetch as a
mirror (refs/*:refs/*), then you would prune everything.

And "--prune-local" doesn't seem like a fetch operation to me. Either
you are mirroring, and --prune already handles it as above. Or you are
interested in getting rid of branches whose upstream has gone away. But
that's not a fetch operation; that's a branch operation.

-Peff

^ 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