Git development
 help / color / mirror / Atom feed
* Re: Bash tab completion for _git_fetch alias is broken on Git 1.7.7.1
From: SZEDER Gábor @ 2011-11-10 15:14 UTC (permalink / raw)
  To: Nathan Broadbent; +Cc: Johannes Sixt, Junio C Hamano, git
In-Reply-To: <CAPXHQbP4yCzZ96WEKuHsV_8Pny0MRzcLOY7qi5W3_L_5CnY0vA@mail.gmail.com>

Hi,

 
[Please don't top-post.]

> On Thu, Nov 10, 2011 at 3:09 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> >
> > Am 11/10/2011 4:21, schrieb Junio C Hamano:
> > > Nathan Broadbent <nathan.f77@gmail.com> writes:
> > >
> > >> Dear git mailing list,
> > >>
> > >> I'm assigning the `_git_fetch` bash tab completion to the alias `gf`,
> > >> with the following command:
> > >>
> > >>     complete -o default -o nospace -F _git_fetch gf

I assume you have an

  alias gf="git fetch"

somewhere, right?

> > >> The tab completion then works fine in git 1.7.0.4, but breaks on git
> > >> 1.7.7.1, with the following error:

I didn't actually tried, but I guess this is a side-effect of da4902a7
(completion: remove unnecessary _get_comp_words_by_ref() invocations,
2011-04-28), which was in v1.7.6.  Since the clean-up in that commit
we only call _get_comp_words_by_ref() in the top-level completion
functions _git() and _gitk() to populate completion-related variables
($cur, $prev, $words, $cword), so invoking any _git_<cmd>() completion
function directly causes an error or wrong behavior, because all those
variables are empty.

Calling a completion function directly was not an issue earlier,
because every _git_<cmd>() completion function invoked
_get_comp_words_by_ref() to populate those variables, or in the
pre-_get_comp_words_by_ref() times they just accessed the
completion-related bash variables $COMP_WORDS and $COMP_CWORD
directly.


On Thu, Nov 10, 2011 at 04:52:33PM +0800, Nathan Broadbent wrote:
> So, this is a feature, not a bug... Tab completion for aliases is
> really useful. It's important enough to me that I won't stop until
> I've found a solution.
> I can appreciate that _git_fetch is not currently meant to be called
> directly, but we found a way to utilize it when it previously worked.
> Perhaps the scope of these completion functions could be expanded to
> allow for aliases? I'll attempt to submit a patch if someone can give
> me approval.

The quickest way would be to just revert da4902a7, but it would be the
dirtiest, too: it would bring back a lot of redundant calls to
_get_comp_words_by_ref() and it might have side-effects under zsh (but
I didn't think this through).

It would be a bit more clever to revert only parts of da4902a7, i.e.
to bring back _get_comp_words_by_ref() calls in _git_<cmd> completion
functions but not in __git_<whatever>() helper functions.  This way
_git_<cmd>() functions would have their completion-related variables
initialized even when called directly instead through _git(), and
_get_comp_words_by_ref() would be called "only" twice during a single
completion.  But that's still one too many, and again: there can be
issues with zsh.

Alternatively, you could easily create your own wrapper function
around _git_fetch(), like this:

_gf () {
	local cur prev words cword
	_get_comp_words_by_ref -n =: cur prev words cword
	_git_fetch
}


However.

Having said all that, I'd like to point out that even if _git_fetch()
didn't error out when called for the 'gf' alias, it still wouldn't
work properly.  After 'gf origin <TAB>' it offers the list of remotes
again and it never offers refspecs, because it calls
__git_complete_remote_or_refspec(), which

 - depends on the fact that there must be at least two words ('git'
   and 'fetch') on the command line before the remote, and

 - needs to know the git command (i.e. fetch, pull, or push) to offer
   the proper refspecs, but it can't find that out from your alias.


Best,
Gábor

^ permalink raw reply

* Re: Bash tab completion for _git_fetch alias is broken on Git 1.7.7.1
From: Nathan Broadbent @ 2011-11-10 14:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <CAPXHQbP2O2C6sDVYLB=eMu0UpdMm79t3fqopqBvNpmdpKPRsXQ@mail.gmail.com>

> No change with 1.7.8-rc1, tab completion still fails

^ permalink raw reply

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: David Woodhouse @ 2011-11-10 13:52 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Junio C Hamano, git, James Bottomley, Jeff Garzik, Andrew Morton,
	linux-ide, LKML
In-Reply-To: <CA+55aFx_rAA6TJkZn1Zvu6u9UjxnmTVt0HpMnvaE_q9Sx-jzPg@mail.gmail.com>

On Tue, 2011-11-01 at 14:21 -0700, Linus Torvalds wrote:
> I hate how anonymous our branches are. Sure, we can use good names for
> them, but it was a mistake to think we should describe the repository
> (for gitweb), rather than the branch.
> 
> Ok, "hate" is a strong word. I don't "hate" it. I don't even think
> it's a major design issue. But I do think that it would have been
> nicer if we had had some branch description model. 

I actually quite like it. I take it as a hint: if the contents of a
branch are *so* wildly different from the main repository that they need
a different description, perhaps I should be using a separate repository
instead of just a branch.

-- 
dwmw2


^ permalink raw reply

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: David Woodhouse @ 2011-11-10 13:51 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Jochen Striepe, Shawn Pearce, Junio C Hamano, git,
	James Bottomley, Jeff Garzik, Andrew Morton, linux-ide, LKML
In-Reply-To: <CA+55aFyG4VuiRN3kcyDVF4sw7b89m-2bOBeQLOGWTcd9o3akzQ@mail.gmail.com>

On Wed, 2011-11-02 at 21:13 -0700, Linus Torvalds wrote:
> No, my main objection to saving the data is that it's ugly and it's
> redundant. Sure, in practice you can check the signatures later fine
> (with the rare exceptions you mention), but even when you can do it,
> what's the big upside? 

Another objection (although it may not be insurmountable) is that it's
not necessarily *entirely* clear what's being signed.

In the simple case where I clone your tree, make a few commits with my
Signed-off-by:, sign a tag and then ask you to pull, that's easy enough.
I'm vouching for what I committed, and not for everything that was in
your tree beforehand.

But what if I'm working on top of someone else's published git tree?
Does a signed tag at the top of *my* work imply that I'm vouching for
all of theirs too?

In the case where the signature is ephemeral and only used for you to
trust my pull request, the answer is simple: If that other work wasn't
in your tree yet at the time I send my pull request, I'd damn well
better be vouching for it when I ask you to pull it. Nothing new there.

But if we're keeping signatures around for auditing purposes, we'd
better have a coherent answer to that question. One that isn't "a
signature cover everything since the last commit with torvalds@ as the
committer", if we want it to be useful for the general case.

-- 
dwmw2


^ permalink raw reply

* RE: Disappearing change on pull rebase
From: Pitucha, Stanislaw Izaak @ 2011-11-10 13:35 UTC (permalink / raw)
  To: git@vger.kernel.org
In-Reply-To: <B5934593-5EE9-4A9F-96D5-0E36B696EFBD@jetbrains.com>

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

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.

$ git init fake_upstream
Initialized empty Git repository in /tmp/fake_upstream/.git/
$ cd fake_upstream/ 
fake_upstream$ echo some_content > test
fake_upstream$ echo 1 > changelog
fake_upstream$ git add test changelog 
fake_upstream$ git commit -m 'first commit'
[master (root-commit) b1b683d] first commit
 2 files changed, 2 insertions(+), 0 deletions(-)
 create mode 100644 changelog
 create mode 100644 test
fake_upstream$ cd ..

$ git clone /tmp/fake_upstream/ disappearing_commit
Cloning into disappearing_commit...
done.
$ cd disappearing_commit/
disappearing_commit$ git checkout -b some-branch
Switched to a new branch 'some-branch'
disappearing_commit$ echo blah >> test
disappearing_commit$ git commit test -m 'branch modification'
[some-branch 528f166] branch modification
 1 files changed, 1 insertions(+), 0 deletions(-)
disappearing_commit$ git checkout master
Switched to branch 'master'

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'

disappearing_commit$ git show
commit e41e4c963a72528e3b2b5945ca419e19cdba1e4b
Merge: b1b683d 528f166
Author: Stanislaw Pitucha <stanislaw.pitucha@hp.com>
Date:   Thu Nov 10 13:30:04 2011 +0000

    Merge branch 'some-branch'

diff --cc changelog
index d00491f,d00491f..1191247
--- a/changelog
+++ b/changelog
@@@ -1,1 -1,1 +1,2 @@@
  1
++2
disappearing_commit$ git pull --rebase
First, rewinding head to replay your work on top of it...
Applying: branch modification
disappearing_commit$ git show
commit ef8f5e40db9fe7efbd4e493ff68b12327acde283
Author: Stanislaw Pitucha <stanislaw.pitucha@hp.com>
Date:   Thu Nov 10 13:29:33 2011 +0000

    branch modification

diff --git a/test b/test
index 6e099dc..2f3f685 100644
--- a/test
+++ b/test
@@ -1 +1,2 @@
 some_content
+blah


Regards,
Stanisław Pitucha
Cloud Services 
Hewlett Packard

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6216 bytes --]

^ permalink raw reply related

* Re: Disappearing change on pull rebase
From: Kirill Likhodedov @ 2011-11-10 14:23 UTC (permalink / raw)
  To: Pitucha, Stanislaw Izaak; +Cc: git@vger.kernel.org
In-Reply-To: <3FF1328CB05DB74898F769F1BA17812C3E49B74671@GVW1348EXA.americas.hpqcorp.net>

10.11.2011, в 16:15, Pitucha, Stanislaw Izaak:

> Hi all,
> I've got an issue with some operations. It seems like git eats one of my commits (it's still in reflog, but in normal tree, it's unavailable).
> 
> What I did is:
> 
> checkout -b feature/....
> (edit files and commit)
> checkout master
> merge --no-ff --no-commit feature/...
> (edit some files, change versions, changelog)
> commit
> 
> Now I've got the change committed in the branch and some more changes on the merge commit.
> So before pushing to the main repo, I'd like to check if any other changes are there:
> 
> pull --rebase
> 
> Now my merge commit disappears completely along with the changes without any warning. I get the branch commits duplicated on top of master and the branch stays as it was.
> That looks like a data loss bug to me since I can only recover a committed change from reflog and there are no warnings before that change goes away (using 1.7.4.1). Actually no changes were done in upstream in the meantime, so the rebase was not even needed.
> 

That is definitely not a bug. 
"git pull --rebase" is (almost?) equal to "git fetch ; git rebase origin/master"
When you perform a rebase, at first your HEAD is rolled back to the commit before your changes, then it is fast-forwared to the remote HEAD (in your case, no fast-forward was made, because there were no remote changes); then your commits are applied one by one.

Of couse, when your commits are applied, they are applied like patches. That mean, that they are different from the original commits (at least, by the commit time). That causes the duplication.

And the merge commit "dissapeared", because it contained no changes, so the patch was empty, and there was nothing to reapply.
If the merge commit contained some changes, and it really was not applied during rebase, it is a bug, but more details will be needed, I think.

If you want to preserve your branch history, you should do "pull" without "rebase".

Kirill.

^ permalink raw reply

* Disappearing change on pull rebase
From: Pitucha, Stanislaw Izaak @ 2011-11-10 13:15 UTC (permalink / raw)
  To: git@vger.kernel.org

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

Hi all,
I've got an issue with some operations. It seems like git eats one of my commits (it's still in reflog, but in normal tree, it's unavailable).

What I did is:

checkout -b feature/....
(edit files and commit)
checkout master
merge --no-ff --no-commit feature/...
(edit some files, change versions, changelog)
commit

Now I've got the change committed in the branch and some more changes on the merge commit.
So before pushing to the main repo, I'd like to check if any other changes are there:

pull --rebase

Now my merge commit disappears completely along with the changes without any warning. I get the branch commits duplicated on top of master and the branch stays as it was.
That looks like a data loss bug to me since I can only recover a committed change from reflog and there are no warnings before that change goes away (using 1.7.4.1). Actually no changes were done in upstream in the meantime, so the rebase was not even needed.

Regards,
Stanisław Pitucha
Cloud Services 
Hewlett Packard



[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6216 bytes --]

^ permalink raw reply

* Re: [fyi] patches used by git distributors
From: Erik Faye-Lund @ 2011-11-10 12:07 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git
In-Reply-To: <20111108090251.GB17954@elie.hsd1.il.comcast.net>

On Tue, Nov 8, 2011 at 10:02 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi,
>
> In an ideal world, each patch applied by downstream distributors would
> fall into one of two categories: (a) adapting the package to some
> esoteric distro-specific requirement (i.e., special-interest patches)
> or (b) in the process of being generalized and reviewed for eventual
> application upstream, so everyone can benefit from it.  Unfortunately
> that takes time.  I should do better --- sorry about that.
>
> As an experiment, here's a quick summary of the patches being used
> in Debian, for people curious about that and for people interested in
> grabbing useful patches to polish and not knowing where to start.
>
> (Links point to relevant discussion, not necessarily the patch used):
>
> Frédéric Brière (1):
>      gitk: Skip over AUTHOR/COMMIT_DATE when searching all fields [1]
>
> Gerrit Pape (1):
>      bug#506445: hooks/post-receive-email: set encoding to utf-8 [2]
>
> Jonathan Nieder (12):
>      remove shebang line from shell libraries [3]
>      pre-rebase hook: capture documentation in a <<here document [4]
>      gitk: use symbolic font names "sans" and "monospace"
>      transport: expose git_tcp_connect and friends in new tcp.h [5]
>      daemon: make host resolution into a separate function [5]
>      daemon: move locate_host to tcp.c [5]
>      tcp: unify ipv4 and ipv6 code paths [5]
>      daemon: check for errors retrieving IP address [5]
>      tcp: make dns_resolve return an error code [5]
>      transport: optionally honor DNS SRV records [5]
>      srv: make errors less quiet [5]
>      Makefile: add a knob to turn off hardlinks within same directory [6]
>
> The patches listed above are on the candidate+patches branch of [7].
> Questions and improvements can go to git@packages.debian.org.
>
> A few other packaging projects:
>
>  - git://pkgs.fedoraproject.org/git.git master --- 3 patches (using
>   SERVER_NAME for home link, reviving vc-git.el, compatibility with
>   newer cvsps)
>  - http://www.freebsd.org/cgi/cvsweb.cgi/ports/devel/git/files/ ---
>   1 patch (capping individual reads and writes at INT_MAX chars)
>  - https://build.opensuse.org/package/files?package=git&project=devel%3Atools%3Ascm
>   --- 4 patches (a python build fix, making gitweb::prevent_xss
>   default to true, turning off hardlinks for builtins at installation
>   time, protecting COMP_WORDBREAKS from mangling in the completion
>   script)
>  - http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/dev-vcs/git/files/
>   --- 1 patch (a NO_CVS knob for the makefile).  Very nice.
>  - http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/devel/scmgit-base/patches/
>   --- 3 patches (putting CFLAGS at the end of ALL_CFLAGS so it can
>   override BASIC_CFLAGS, setting INSTALLDIRS=vendor in perl makefile,
>   improving tk support on Darwin 8)
>  - http://www.openbsd.org/cgi-bin/cvsweb/ports/devel/git/patches/ ---
>   8 patches (updating OpenBSD makefile defaults, using raw perlio in
>   gitweb blob view, removing "set -e" in t9117, passing --text [well,
>   -a] to grep in t9200, avoiding nonportable regex \+ in t9400)
>  - ftp://ftp.cygwin.org/pub/cygwin/release/git/git-1.7.5.1-1-src.tar.bz2
>   --- 3 patches (tcl 8.4.1 support, updating Cygwin makefile defaults,
>   case-insensitive path comparison in makefile, special Windows-specific
>   wish script preamble)
>

Here's what we have for msysGit (the commits themselves can be found
in git://github.com/msysgit/git.git devel):

$ git log --no-merges --first-parent --oneline 319312f..origin/devel
9e47e31 Unicode console: fix font warning on Vista and Win7
778774d MSVC: require pton and ntop emulation
a49c818 MSVC: fix winansi.c compile errors
9ca8039 MSVC: fix poll-related macro redefines
0a6d148 MSVC: Remove unneeded header stubs
18f5b9e Compile fix for MSVC: Include <io.h>
fd6f6aa Compile fix for MSVC: Do not include sys/resources.h
ae69aaf Handle the branch.<name>.rebase value 'interactive'
5d1168b Teach 'git pull' to handle --rebase=interactive
5d6f1d9 Windows: define S_ISUID properly
5084033 Fixed wrong path delimiter in exe finding
77a30da gitweb (SyntaxHighlighter): interpret #l<line-number>
4654bdd Only switch on the line number toggle when highlighting is activated
3efcd98 Gitweb: add support for Alex Gorbatchev's SyntaxHighlighter in
Javascript
1d9ac66 MinGW: disable CRT command line globbing
e0f2492 Win32: move main macro to a function
8258c82 Win32: fix potential multi-threading issue
c45b052 Win32 dirent: improve dirent implementation
ab7ab7a Win32 dirent: clarify #include directives
9d46513 Win32 dirent: change FILENAME_MAX to MAX_PATH
18cf321 Win32 dirent: remove unused dirent.d_reclen member
2741409 Win32 dirent: remove unused dirent.d_ino member
a27af5c Revert "MinGW: Add missing file mode bit defines"
3e4b790 submodule: Fix t7400, t7405, t7406 for msysGit
f72cc9a t5407: Fix line-ending dependency in post-rewrite.args
77de716 submodule: Use cat instead of echo to avoid DOS line-endings
fece0cb t3102: Windows filesystems may not use a literal asterisk in filenames.
d3abd84 Disable test on MinGW that challenges its bash quoting
3f65607 MinGW: Skip test redirecting to fd 4
2e1f250 mingw: do not hide bare repositories
8089c85c Git.pm: Use stream-like writing in cat_blob()
d3d677e Amend "git grep -O -i: if the pager is 'less', pass the '-i' option"
ed7b275 git grep -O -i: if the pager is 'less', pass the '-i' option
30d3f9e Handle new t1501 test case properly with MinGW
64f7bec Do not compile compat/**/*.c with -Wold-style-definition
68acf50 Fix old-style function declaration
aa1c11f Make CFLAGS more strict
c592e94 grep -I: do not bother to read known-binary files
352f3d0 Let deny.currentBranch=updateInstead ignore submodules
5cf00b3 add -e: ignore dirty submodules
bf239f8 Handle http.* config variables pointing to files gracefully on Windows
621a958 Gitweb: make line number toggling work for Firefox and Safari
ff19df4 gitweb: Allow line number toggling with Javascript
feab003 Give commit message reencoding for output on MinGW a chance
6f86c3e Warn if the Windows console font doesn't support Unicode
bfcdc94 Detect console streams more reliably on Windows
9b505ab Support Unicode console output on Windows
e3356ac Enable color output in Windows cmd.exe
3bef266 Fix another invocation of git from gitk with an overly long command-line
d0a4bbe git gui: set GIT_ASKPASS=git-gui--askpass if not set yet
7602e52 Make mingw_offset_1st_component() behave consistently for all paths.
1c072a6 config.c: trivial fix for compile-time warning
1f3a4f8 Allow using UNC path for git repository
3d08379 work around misdetection of stdin attached to a tty
fef92f8 git am: ignore dirty submodules
d23ce6f t7602: cope with CR/LF
3649391 Add a Windows-specific fallback to getenv("HOME");
ede54b1 send-email: handle Windows paths for display just like we do
for processing
32eefc4 send-email: accept absolute path even on Windows
6274596 Add a few more values for receive.denyCurrentBranch
2afff36 Work around funny CR issue
b81e593 mingw: add tests for the hidden attribute on the git directory
f9445e4 Work around the command line limit on Windows
59e0e60 git-gui: provide question helper for retry fallback on Windows
9e89381 Revert "git-gui: set GIT_DIR and GIT_WORK_TREE after setup"
40e6ef7 criss cross rename failure workaround
d24ec01 When initializing .git/, record the current setting of core.hideDotFiles
c40046b core.hidedotfiles: hide '.git' dir by default
73afc46 MinGW: Add missing file mode bit defines

Some of these might be useful for upstream as well...

^ permalink raw reply

* Re: RFH: unexpected reflog behavior with --since=
From: Miles Bader @ 2011-11-10 11:06 UTC (permalink / raw)
  To: Jeff King; +Cc: Eric Raible, git@vger.kernel.org
In-Reply-To: <20111110080851.GA28342@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:
> The only point would be to leave "--since" to act on the commit
> timestamps, so that you don't have to resort to the external grepping I
> mentioned above. However, I'm not convinced anybody even cares about
> that use case.
>
> I think the behavior you want is much more sensible.

I think there's already confusion in this area, e.g., with @{...} using
reflog dates, but "git log --since" using commit dates.  This can be an
easy trap to fall into because _often_ the two have similar granularity
(when you're mostly pushing changes), but not _always_ (when you pull a
big batch of changes).

Soooo, being really really explicit about using reflog dates vs. commit
dates -- and e.g., having option names like "--since" _always_ refer to
commit dates -- would be a good thing, I think...

-Miles

-- 
Future, n. That period of time in which our affairs prosper, our friends
are true and our happiness is assured.

^ permalink raw reply

* [PATCH] mktree: fix a memory leak in write_tree()
From: Liu Yuan @ 2011-11-10  9:04 UTC (permalink / raw)
  To: 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: Bash tab completion for _git_fetch alias is broken on Git 1.7.7.1
From: Nathan Broadbent @ 2011-11-10  8:52 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, git
In-Reply-To: <4EBB78C7.101@viscovery.net>

So, this is a feature, not a bug... Tab completion for aliases is
really useful. It's important enough to me that I won't stop until
I've found a solution.
I can appreciate that _git_fetch is not currently meant to be called
directly, but we found a way to utilize it when it previously worked.
Perhaps the scope of these completion functions could be expanded to
allow for aliases? I'll attempt to submit a patch if someone can give
me approval.


Thanks,
Nathan


On Thu, Nov 10, 2011 at 3:09 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>
> Am 11/10/2011 4:21, schrieb Junio C Hamano:
> > Nathan Broadbent <nathan.f77@gmail.com> writes:
> >
> >> Dear git mailing list,
> >>
> >> I'm assigning the `_git_fetch` bash tab completion to the alias `gf`,
> >> with the following command:
> >>
> >>     complete -o default -o nospace -F _git_fetch gf
> >>
> >> The tab completion then works fine in git 1.7.0.4, but breaks on git
> >> 1.7.7.1, with the following error:
> >
> > We have been cooking for 1.7.8 and have the first release candidate
> > 1.7.8-rc1; could you try it and report what you find out?
>
> It looks like _git_fetch is not meant to be called directly. All git
> completions must go through _git.
>
> See also this post:
>
> http://thread.gmane.org/gmane.comp.version-control.msysgit/13310/focus=13335
>
> -- Hannes

^ permalink raw reply

* Re: [PATCH 02/14] http: turn off curl signals
From: Daniel Stenberg @ 2011-11-10  8:43 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20111110074803.GB27950@sigill.intra.peff.net>

On Thu, 10 Nov 2011, Jeff King wrote:

> I'm a little iffy on this one. If I understand correctly, depending on the 
> build and configuration, curl may not be able to timeout during DNS lookups. 
> But I'm not sure if it does, anyway, since we don't set any timeouts.

Right, without a timeout set libcurl won't try to timeout name resolves.

To clarify: when libcurl is built to use the standard synchronous name 
resolver functions it can only abort them after a specified time by using 
signals (on posix systems).

-- 

  / daniel.haxx.se

^ permalink raw reply

* [PATCH] mktree: fix a memory leak in write_tree()
From: Liu Yuan @ 2011-11-10  8:39 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: Problem with git-svn with limited SVN access
From: Antoine Bonavita @ 2011-11-10  8:36 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git
In-Reply-To: <201111092338.26164.trast@student.ethz.ch>

Thomas,

Thanks a lot for taking the time to look at my problem.

On 11/09/2011 11:38 PM, Thomas Rast wrote:
> Antoine Bonavita wrote:
>> ### If I try to add one of the branches manually:
>> branches = branches/XXX:refs/remotes/branches/XXX
>>   >  git svn fetch
>> One '*' is needed in glob: 'branches/XXX'
>
> I think having several fetch specs should do the trick, although I
> cannot easily test with actual permissions.
>
> You can start configuring the repo with
>
>    git init
>    git svn init svn://server/ -T trunk
>
> to get an initial layout.  The .git/config will look like
>
>    [svn-remote "svn"]
>            url = svn://server/
>            fetch = trunk:refs/remotes/trunk
>
> The clue is that the config says 'fetch', not 'trunk'.  Much like with
> git remotes, you can add more fetch specs along the lines of
>
>            fetch = branches/XXX:refs/remotes/svn/XXX
>
> or whatever layout you prefer.
I did what you suggested. Now my .git/config looks like:
[svn-remote "svn"]
         url = svn://server/aaa/AAA
         fetch = trunk:refs/remotes/trunk
        fetch = branches/XXX:refs/remotes/svn/XXX
  and here is the result:
 > git svn fetch
W: Item is not readable: Item is not readable at 
/usr/libexec/git-core/git-svn line 1782

Error from SVN, (220001): Item is not readable: Item is not readable

Any other idea ?
Or is there a way to get more debug info from git-svn. May be it would 
give us some insight on what it is trying to do and failing to.

Thanks,

Antoine.

>
> Please tell us whether that works even in the face of restrictions on
> branches/ itself :-)
>

-- 
Antoine Bonavita (antoine@stickyadstv.com)
Envoyé de mon PC. Moi je suis Fedora.

^ permalink raw reply

* Re: RFH: unexpected reflog behavior with --since=
From: Jay Soffian @ 2011-11-10  8:31 UTC (permalink / raw)
  To: Jeff King; +Cc: Eric Raible, git@vger.kernel.org
In-Reply-To: <20111110080851.GA28342@sigill.intra.peff.net>

On Thu, Nov 10, 2011 at 3:08 AM, Jeff King <peff@peff.net> wrote:
> it to Junio. But also, I'd like to gather more opinions on whether the
> design is the right thing (hopefully the implementation is Obviously

Makes sense to me, so you're at +3.

j.

^ permalink raw reply

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Johan Herland @ 2011-11-10  8:02 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Linus Torvalds, Ted Ts'o, Shawn Pearce, git, James Bottomley,
	Jeff Garzik, Andrew Morton, linux-ide, LKML
In-Reply-To: <7v4nydurzh.fsf@alter.siamese.dyndns.org>

On Wed, Nov 9, 2011 at 18:26, Junio C Hamano <gitster@pobox.com> wrote:
>  - "git notes" is represented as a commit that records a tree that holds
>   the entire mapping from commit to its annotations, and the only way to
>   transferr it is to send it together with its history as a whole. It
>   does not have the nice auto-following property that transfers only the
>   relevant annotations.

True. However, consider these mitigating factors:

 - The annotations in question (the "signing" of commits) are all intended to
   be merged eventually (i.e. there is no reason for a developer to (after the
   fact) sign a commit that will never end up in the public record). Therefore,
   most or all of the notes in the notes tree are already relevant, or
will become
   relevant in the near future (when the associated commits are merged).

 - Additionally, you could organize these notes into two (or more) notes trees,
   one for merged/official annotations, and one for unmerged/pending
annotations.
   Then make the relevant tools (e.g. "git merge") transfer notes from one tree
   to the other, thereby making sure that the "official" record only contains
   notes that are relevant to the merged history.

 - Finally, there's always "git notes prune" to purge annotations for commits
   that ended up never being merged.

My point is that although "notes" might end up transferring more annotations
than strictly necessary, I believe that in practice all the notes
being transferred
are already (or will soon become) relevant.

>  + "git notes" maps the commits to its annotations in the right direction;
>   the object name of an annotated object to its annotation.
>
> In the longer term, I think we would need to extend the system in the
> following way:
>
>  - Introduce a mapping machanism that can be locally used to map names of
>   the objects being annotated to names of other objects (most likely
>   blobs but there is nothing that fundamentally prevents you from
>   annotating a commit with a tree). The current "git notes" might be a
>   perfectly suitable representation of this, or it may turn out to be
>   lacking (I haven't thought things through), but the important point is
>   that this "mapping store" is _local_. fsck, repack and prune need to be
>   told that objects that store the annotation are reachable from the
>   annotated objects.

IMHO this is precisely what "git notes" does today.

>  - Introduce a protocol extension to transfer this mapping information for
>   objects being transferred in an efficient way. When "rev-list --objects
>   have..want" tells us that the receiving end (in either fetch/push
>   direction) would have an object at the end of the primary transfer
>   (note that I did not say "an object will be sent in this transfer
>   transaction"; "have" does not come into the picture), we make sure that
>   missing annotations attached to the object is also transferred, and new
>   mapping is registered at the receiving end.
>
> The detailed design for the latter needs more thought. The auto-following
> of tags works even if nothing is being fetched in the primary transfer
> (i.e. "git fetch" && "git fetch" back to back to update our origin/master
> with the master at the origin) when a new tag is added to ancient part of
> the history that leads to the master at the origin, but this is exactly
> because the sending end advertises all the available tags and the objects
> they point at so that we can tell what new tags added to an old object is
> missing from the receiving end. This obviously would not scale well when
> we have tens of thousands of objects to annotate. Perhaps an entry in the
> "mapping store" would record:
>
>  - The object name of the object being annotated;
>
>  - The object name of the annotation;
>
>  - The "timestamp", i.e. when the association between the above two was
>   made--this can be local to the repository and a simple counter would
>   do.
>
> and also maintain the last "timestamp" this repository sent annotations to
> the remote (one timestamp per remote repository). When we push, we would
> send annotations pertaining to the object reachable from what we are
> pushing (not limited by what they already have, as the whole point of this
> exercise is to allow us to transfer annotations added to an object long
> after the object was created and sent to the remote) that is newer than
> that "timestamp". Similarly, when fetching, we would send the "timestamp"
> this repository last fetched annotations from the other end (which means
> we would need one such "timestamp" per remote repository) and let the
> remote side decide the set of new annotations they added since we last
> synched that are on objects reachable from what we "want".
>
> Or something like that.

You would also have to keep track of deleted annotations, to enable the local
side to delete an annotation corresponding to an already-deleted annotation
on the remote side.

Pretty soon, you end up having to record something similar to a DAG,
describing the history of manipulating these annotations. At that point, your
"timestamp" calculation starts to look very similar to the "have..want"
calculation already done when transferring "regular" refs. At which point you
have a system that is very similar to what "git notes" does today...


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* Re: RFH: unexpected reflog behavior with --since=
From: Eric Raible @ 2011-11-10  8:20 UTC (permalink / raw)
  To: Jeff King; +Cc: git@vger.kernel.org
In-Reply-To: <20111110080851.GA28342@sigill.intra.peff.net>

On 11/10/2011 12:08 AM, Jeff King wrote:
>>>   git log -g --format='%ct %H' |
>>>   awk '{ print $2 if $1 < SOME_TIMESTAMP }'
>>
>> And then the sha would have to be fed back into git to be useful, eh?
> 
> It's just illustrative. You could replace "%H" with the actual
> information you're interested in.

Of course, my thinko.

> The only point would be to leave "--since" to act on the commit
> timestamps, so that you don't have to resort to the external grepping I
> mentioned above. However, I'm not convinced anybody even cares about
> that use case.
> 
> I think the behavior you want is much more sensible.

Me too!

>> Is this something you'd be willing to turn into a real patch?
>> I'm certainly not qualified.
> 
> Yes. We're in release freeze now, so I didn't even bother with sending
> it to Junio. But also, I'd like to gather more opinions on whether the
> design is the right thing (hopefully the implementation is Obviously
> Correct. :) ).

I think it's hard to argue that the current behavior (as illustrated with
my original example) makes sense.  Or that your patch is overly complicated.
But giving people time to chime in it definitely TRTTD.

- Eric

^ permalink raw reply

* Re: RFH: unexpected reflog behavior with --since=
From: Jeff King @ 2011-11-10  8:08 UTC (permalink / raw)
  To: Eric Raible; +Cc: git@vger.kernel.org
In-Reply-To: <4EBB8596.6040507@nextest.com>

On Thu, Nov 10, 2011 at 12:04:38AM -0800, Eric Raible wrote:

> > I think I am leaning towards the latter. It seems to me to be the more
> > likely guess for what the user would want. And there is real benefit to
> > doing it in git, since we can stop the traversal early. In the
> > "grep-like" case, doing it inside git is not really any more efficient
> > than filtering in a pipeline, like:
> > 
> >   git log -g --format='%ct %H' |
> >   awk '{ print $2 if $1 < SOME_TIMESTAMP }'
> 
> And then the sha would have to be fed back into git to be useful, eh?

It's just illustrative. You could replace "%H" with the actual
information you're interested in.

> > Of course we could still offer both (with a "--reflog-since" type of
> > option). We'd also need to turn off the optimization for "--since", and
> > then check whether "--until" has a similar bug (and offer
> > "--reflog-until").
> 
> I don't see the point of --reflog-since.  If the user specifies 'reflog'
> (either directly or with -g), then can't we just use the reflog's timestamp?
> Note: there might be good reasons, as my use of the reflog (and --since, for
> that matter), has been very simplistic so far.

The only point would be to leave "--since" to act on the commit
timestamps, so that you don't have to resort to the external grepping I
mentioned above. However, I'm not convinced anybody even cares about
that use case.

I think the behavior you want is much more sensible.

> > diff --git a/reflog-walk.c b/reflog-walk.c
> > index 5d81d39..2e5b270 100644
> > --- a/reflog-walk.c
> > +++ b/reflog-walk.c
> > @@ -231,6 +231,7 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
> >  	reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
> >  	info->last_commit_reflog = commit_reflog;
> >  	commit_reflog->recno--;
> > +	commit->date = reflog->timestamp;
> >  	commit_info->commit = (struct commit *)parse_object(reflog->osha1);
> >  	if (!commit_info->commit) {
> >  		commit->parents = NULL;
> 
> Is this something you'd be willing to turn into a real patch?
> I'm certainly not qualified.

Yes. We're in release freeze now, so I didn't even bother with sending
it to Junio. But also, I'd like to gather more opinions on whether the
design is the right thing (hopefully the implementation is Obviously
Correct. :) ).

-Peff

^ permalink raw reply

* Re: RFH: unexpected reflog behavior with --since=
From: Eric Raible @ 2011-11-10  8:04 UTC (permalink / raw)
  To: Jeff King; +Cc: git@vger.kernel.org
In-Reply-To: <20111109222032.GB31535@sigill.intra.peff.net>

On 11/9/2011 2:20 PM, Jeff King wrote:
> On Wed, Nov 09, 2011 at 05:01:28PM -0500, Jeff King wrote:
> 
> This patch (which is below) turns out to be absurdly simple. And it
> actually still prints the original commit timestamp, because we end up
> reparsing it out of the commit object during the pretty-print phase.

Sweet!

> So I think the only decision is whether "--since" should respect the
> commit timestamps (and be used as a sort of "grep" filter for
> timestamps), or whether it should be respecting the fake history we
> create when doing a reflog walk.

When -g is specified it seems less surprising for --since to respect
the reflog's fake history.  That's what *I* expected, anyway.

> I think I am leaning towards the latter. It seems to me to be the more
> likely guess for what the user would want. And there is real benefit to
> doing it in git, since we can stop the traversal early. In the
> "grep-like" case, doing it inside git is not really any more efficient
> than filtering in a pipeline, like:
> 
>   git log -g --format='%ct %H' |
>   awk '{ print $2 if $1 < SOME_TIMESTAMP }'

And then the sha would have to be fed back into git to be useful, eh?

> Of course we could still offer both (with a "--reflog-since" type of
> option). We'd also need to turn off the optimization for "--since", and
> then check whether "--until" has a similar bug (and offer
> "--reflog-until").

I don't see the point of --reflog-since.  If the user specifies 'reflog'
(either directly or with -g), then can't we just use the reflog's timestamp?
Note: there might be good reasons, as my use of the reflog (and --since, for
that matter), has been very simplistic so far.

> diff --git a/reflog-walk.c b/reflog-walk.c
> index 5d81d39..2e5b270 100644
> --- a/reflog-walk.c
> +++ b/reflog-walk.c
> @@ -231,6 +231,7 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
>  	reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
>  	info->last_commit_reflog = commit_reflog;
>  	commit_reflog->recno--;
> +	commit->date = reflog->timestamp;
>  	commit_info->commit = (struct commit *)parse_object(reflog->osha1);
>  	if (!commit_info->commit) {
>  		commit->parents = NULL;

Is this something you'd be willing to turn into a real patch?
I'm certainly not qualified.

Thanks - Eric

^ permalink raw reply

* Re: RFH: unexpected reflog behavior with --since=
From: Jeff King @ 2011-11-10  7:59 UTC (permalink / raw)
  To: Eric Raible; +Cc: git@vger.kernel.org
In-Reply-To: <4EBB81EA.6060303@nextest.com>

On Wed, Nov 09, 2011 at 11:48:58PM -0800, Eric Raible wrote:

> > But it may also be a misfeature, because it's not clear what you're
> > actually trying to limit by. We have commit timestamps, of course, but
> > when we are walking reflogs, we also have reflog timestamps. Did you
> > actually want to say "show me all commits in the reflog, in reverse
> > reflog order, omitting commits that happened before time t"? Or did you
> > really mean "show me the reflog entries that happened before time t,
> > regardless of their commit timestamp"?
> 
> I meant "show me the reflog entries that happened *since* time t,
> regardless of their commit timestamp.

Err, yeah, sorry. Somehow in the middle of writing the email I got
turned backwards about which direction we were interested in.

But I think you get the point.

> Since -g is asking specifying for the reflog, and since the reflog has
> its own timestamps, I would expect that those timestamps be used.

Then I think my one-liner patch should do what you want. And now it's
not just anecdotal evidence that I think it's the right behavior. There
are two of us; we're _data_.

-Peff

^ permalink raw reply

* [PATCH 14/14] clone: give advice on how to resume a failed clone
From: Jeff King @ 2011-11-10  7:56 UTC (permalink / raw)
  To: git
In-Reply-To: <20111110074330.GA27925@sigill.intra.peff.net>

When clone fails, we usually delete the partial directory.
However, if cloning was fetching a bundle, that is resumable
and we should consider those results precious.

This patch detects when a partial bundle is present,
preserves the directory, and gives the user some advice
about how to resume.

Signed-off-by: Jeff King <peff@peff.net>
---
We could make "git clone ..." automatically resume, but I'm a little
nervous about that. I wrote a patch that did so, and it did work, but
there are a lot of little hiccups as we violate the assumption that the
directory didn't already exist (e.g., it writes multiple fetch refspec
lines to the config).

But more importantly, I really worry about destroying the safety valve
of not overwriting an existing directory. Yes, we can check to see that
it is a git directory and that it has a partially-downloaded bundle
file. But that could also describe an existing git repo with unstaged
changes. And you sure don't want to "checkout -f" over them.

So I'd rather at least start with giving the user some advice and
having them explicitly say "yeah, I do want to resume this".

 builtin/clone.c |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index efe8b6c..c242e20 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -392,6 +392,36 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
 	return ret;
 }
 
+static int git_dir_is_resumable(const char *dir)
+{
+	const char *objects = mkpath("%s/objects", dir);
+	DIR *dh = opendir(objects);
+	struct dirent *de;
+
+	if (!dh)
+		return 0;
+
+	while ((de = readdir(dh))) {
+		if (!prefixcmp(de->d_name, "tmp_bundle_")) {
+			closedir(dh);
+			return 1;
+		}
+	}
+
+	closedir(dh);
+	return 0;
+}
+
+static void give_resume_advice(void)
+{
+	advise("Cloning failed, but partial results were saved.");
+	advise("You can resume the fetch with:");
+	advise("  git fetch");
+	if (!option_bare)
+		advise("  git checkout %s",
+		       option_branch ? option_branch : "master");
+}
+
 static const char *junk_work_tree;
 static const char *junk_git_dir;
 static pid_t junk_pid;
@@ -402,6 +432,10 @@ static void remove_junk(void)
 	if (getpid() != junk_pid)
 		return;
 	if (junk_git_dir) {
+		if (git_dir_is_resumable(junk_git_dir)) {
+			give_resume_advice();
+			return;
+		}
 		strbuf_addstr(&sb, junk_git_dir);
 		remove_dir_recursively(&sb, 0);
 		strbuf_reset(&sb);
-- 
1.7.7.2.7.g9f96f

^ permalink raw reply related

* [PATCH 13/14] remote-curl: resume interrupted bundle transfers
From: Jeff King @ 2011-11-10  7:53 UTC (permalink / raw)
  To: git
In-Reply-To: <20111110074330.GA27925@sigill.intra.peff.net>

If we have a bundle file from a previous fetch that matches
this URL, then we should resume the transfer where we left
off.

Signed-off-by: Jeff King <peff@peff.net>
---
The second half of the diff is hard to read because I re-indent a big
chunk. It's much easier to see what's going on with "diff -b".

 remote-curl.c |   64 +++++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 6b0820e..43ad1b6 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -9,6 +9,7 @@
 #include "sideband.h"
 #include "bundle.h"
 #include "progress.h"
+#include "dir.h"
 
 static struct remote *remote;
 static const char *url; /* always ends with a trailing slash */
@@ -171,6 +172,32 @@ static int get_refs_from_url(const char *url, struct strbuf *out, int options,
 	return ret;
 }
 
+static int resume_bundle(const char *url, const char *tmpname)
+{
+	struct get_refs_cb_data data;
+	int ret;
+
+	data.fh = fopen(tmpname, "ab");
+	if (!data.fh)
+		die_errno("unable to open %s", tmpname);
+
+	data.is_bundle = 1;
+	data.total = ftell(data.fh);
+	if (options.progress) {
+		data.progress = start_progress("Resuming bundle", 0);
+		display_progress(data.progress, 0);
+		display_throughput(data.progress, data.total);
+	}
+
+	ret = http_get_callback(url, get_refs_callback, &data, data.total, 0);
+
+	if (fclose(data.fh))
+		die_errno("unable to write to %s", tmpname);
+	stop_progress(&data.progress);
+
+	return ret;
+}
+
 static const char *url_to_bundle_tmpfile(const char *url)
 {
 	struct strbuf buf = STRBUF_INIT;
@@ -210,20 +237,35 @@ static int get_refs_from_url(const char *url, struct strbuf *out, int options,
 	free_discovery(last);
 
 	filename = url_to_bundle_tmpfile(url);
+	if (file_exists(filename)) {
+		struct strbuf trimmed = STRBUF_INIT;
 
-	strbuf_addf(&buffer, "%sinfo/refs", url);
-	if (!prefixcmp(url, "http://") || !prefixcmp(url, "https://")) {
-		is_http = 1;
-		if (!strchr(url, '?'))
-			strbuf_addch(&buffer, '?');
-		else
-			strbuf_addch(&buffer, '&');
-		strbuf_addf(&buffer, "service=%s", service);
+		strbuf_addstr(&trimmed, url);
+		while (trimmed.len > 0 && trimmed.buf[trimmed.len-1] == '/')
+			strbuf_setlen(&trimmed, trimmed.len - 1);
+		refs_url = strbuf_detach(&trimmed, NULL);
+
+		http_ret = resume_bundle(refs_url, filename);
+		is_bundle = 1;
 	}
-	refs_url = strbuf_detach(&buffer, NULL);
+	else
+		http_ret = HTTP_MISSING_TARGET;
+
+	if (http_ret != HTTP_OK && http_ret != HTTP_NOAUTH) {
+		strbuf_addf(&buffer, "%sinfo/refs", url);
+		if (!prefixcmp(url, "http://") || !prefixcmp(url, "https://")) {
+			is_http = 1;
+			if (!strchr(url, '?'))
+				strbuf_addch(&buffer, '?');
+			else
+				strbuf_addch(&buffer, '&');
+			strbuf_addf(&buffer, "service=%s", service);
+		}
+		refs_url = strbuf_detach(&buffer, NULL);
 
-	http_ret = get_refs_from_url(refs_url, &buffer, HTTP_NO_CACHE,
-				     filename, &is_bundle);
+		http_ret = get_refs_from_url(refs_url, &buffer, HTTP_NO_CACHE,
+					     filename, &is_bundle);
+	}
 
 	/* try again with "plain" url (no ? or & appended) */
 	if (http_ret != HTTP_OK && http_ret != HTTP_NOAUTH) {
-- 
1.7.7.2.7.g9f96f

^ permalink raw reply related

* [PATCH 12/14] remote-curl: show progress for bundle downloads
From: Jeff King @ 2011-11-10  7:53 UTC (permalink / raw)
  To: git
In-Reply-To: <20111110074330.GA27925@sigill.intra.peff.net>

Generally, the point of fetching from a bundle is that it's
big. Without a progress meter, git will appear to hang
during the long download.

This patch adds a throughput meter (i.e., just the bytes
transferred and the rate). In the long run, we should look
for a content-length header from the server so we can show a
total size and completion percentage. However, displaying
that properly will require some surgery to the progress
code, so let's leave it as a future enhancement.

Signed-off-by: Jeff King <peff@peff.net>
---
 remote-curl.c |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 7734495..6b0820e 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -8,6 +8,7 @@
 #include "pkt-line.h"
 #include "sideband.h"
 #include "bundle.h"
+#include "progress.h"
 
 static struct remote *remote;
 static const char *url; /* always ends with a trailing slash */
@@ -107,6 +108,9 @@ struct get_refs_cb_data {
 	int is_bundle;
 	const char *tmpname;
 	FILE *fh;
+
+	struct progress *progress;
+	off_t total;
 };
 
 static size_t get_refs_callback(char *buf, size_t sz, size_t n, void *vdata)
@@ -114,8 +118,11 @@ static size_t get_refs_callback(char *buf, size_t sz, size_t n, void *vdata)
 	struct get_refs_cb_data *data = vdata;
 	struct strbuf *out = data->out;
 
-	if (data->is_bundle > 0)
+	if (data->is_bundle > 0) {
+		data->total += sz * n;
+		display_throughput(data->progress, data->total);
 		return fwrite(buf, sz, n, data->fh);
+	}
 
 	strbuf_add(out, buf, sz * n);
 
@@ -129,6 +136,12 @@ static size_t get_refs_callback(char *buf, size_t sz, size_t n, void *vdata)
 			die_errno("unable to open %s", data->tmpname);
 		if (fwrite(out->buf, 1, out->len, data->fh) < out->len)
 			die_errno("unable to write to %s", data->tmpname);
+		if (options.progress) {
+			data->total = out->len;
+			data->progress = start_progress("Downloading bundle", 0);
+			display_progress(data->progress, 0);
+			display_throughput(data->progress, data->total);
+		}
 	}
 	return sz * n;
 }
@@ -143,6 +156,8 @@ static int get_refs_from_url(const char *url, struct strbuf *out, int options,
 	data.is_bundle = -1;
 	data.tmpname = tmpname;
 	data.fh = NULL;
+	data.progress = NULL;
+	data.total = 0;
 
 	ret = http_get_callback(url, get_refs_callback, &data, 0, options);
 
@@ -150,6 +165,7 @@ static int get_refs_from_url(const char *url, struct strbuf *out, int options,
 		if (fclose(data.fh))
 			die_errno("unable to write to %s", data.tmpname);
 	}
+	stop_progress(&data.progress);
 
 	*is_bundle = data.is_bundle > 0;
 	return ret;
-- 
1.7.7.2.7.g9f96f

^ permalink raw reply related

* [PATCH 11/14] progress: allow pure-throughput progress meters
From: Jeff King @ 2011-11-10  7:53 UTC (permalink / raw)
  To: git
In-Reply-To: <20111110074330.GA27925@sigill.intra.peff.net>

The progress code assumes we are counting something (usually
objects), even if we are measuring throughput. This works
for fetching packfiles, since they show us the object count
alongside the throughput, like:

  Receiving objects:   2% (301/11968), 22.00 MiB | 10.97 MiB/s

You can also tell the progress code you don't know how many
items you have (by specifying a total of 0), and it looks
like:

  Counting objects: 34957

However, if you're fetching a single large item, you want
throughput but you might not have a meaningful count. You
can say you are getting item 0 or 1 out of 1 total, but then
the percent meter is misleading:

  Downloading:   0% (0/1), 22.00 MiB | 10.97 MiB/s

or

  Downloading: 100% (0/1), 22.00 MiB | 10.97 MiB/s

Neither of those is accurate. You are probably somewhere
between zero and 100 percent through the operation, but you
don't know how far.

Telling it you don't know how many items is even uglier:

  Downloading: 1, 22.00 MiB | 10.97 MiB/s

Instead, this patch will omit the count entirely if you are
on the zero-th item of an unknown number of items. It looks
like:

  Downloading: 22.00 MiB | 10.97 MiB/s

Signed-off-by: Jeff King <peff@peff.net>
---
This was the last amount of work to massage the progress code into doing
what I wanted. It might be nicer if it could show a percentage (if we
know the total size), but there's even more surgery required for that.

 progress.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/progress.c b/progress.c
index 3971f49..92fc3c5 100644
--- a/progress.c
+++ b/progress.c
@@ -103,7 +103,10 @@ static int display(struct progress *progress, unsigned n, const char *done)
 			return 1;
 		}
 	} else if (progress_update) {
-		fprintf(stderr, "%s: %u%s%s", progress->title, n, tp, eol);
+		fprintf(stderr, "%s: ", progress->title);
+		if (n)
+			fprintf(stderr, "%u", n);
+		fprintf(stderr, "%s%s", tp, eol);
 		fflush(stderr);
 		progress_update = 0;
 		return 1;
@@ -113,23 +116,24 @@ static int display(struct progress *progress, unsigned n, const char *done)
 }
 
 static void throughput_string(struct throughput *tp, off_t total,
-			      unsigned int rate)
+			      unsigned int rate, struct progress *p)
 {
+	const char *delim = p->total || p->last_value > 0 ? ", " : "";
 	int l = sizeof(tp->display);
 	if (total > 1 << 30) {
-		l -= snprintf(tp->display, l, ", %u.%2.2u GiB",
+		l -= snprintf(tp->display, l, "%s%u.%2.2u GiB", delim,
 			      (int)(total >> 30),
 			      (int)(total & ((1 << 30) - 1)) / 10737419);
 	} else if (total > 1 << 20) {
 		int x = total + 5243;  /* for rounding */
-		l -= snprintf(tp->display, l, ", %u.%2.2u MiB",
+		l -= snprintf(tp->display, l, "%s%u.%2.2u MiB", delim,
 			      x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
 	} else if (total > 1 << 10) {
 		int x = total + 5;  /* for rounding */
-		l -= snprintf(tp->display, l, ", %u.%2.2u KiB",
+		l -= snprintf(tp->display, l, "%s%u.%2.2u KiB", delim,
 			      x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
 	} else {
-		l -= snprintf(tp->display, l, ", %u bytes", (int)total);
+		l -= snprintf(tp->display, l, "%s%u bytes", delim, (int)total);
 	}
 
 	if (rate > 1 << 10) {
@@ -197,7 +201,7 @@ void display_throughput(struct progress *progress, off_t total)
 		tp->last_misecs[tp->idx] = misecs;
 		tp->idx = (tp->idx + 1) % TP_IDX_MAX;
 
-		throughput_string(tp, total, rate);
+		throughput_string(tp, total, rate, progress);
 		if (progress->last_value != -1 && progress_update)
 			display(progress, progress->last_value, NULL);
 	}
@@ -255,7 +259,7 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)
 		if (tp) {
 			unsigned int rate = !tp->avg_misecs ? 0 :
 					tp->avg_bytes / tp->avg_misecs;
-			throughput_string(tp, tp->curr_total, rate);
+			throughput_string(tp, tp->curr_total, rate, progress);
 		}
 		progress_update = 1;
 		sprintf(bufp, ", %s.\n", msg);
-- 
1.7.7.2.7.g9f96f

^ permalink raw reply related

* [PATCH 10/14] remote-curl: try base $URL after $URL/info/refs
From: Jeff King @ 2011-11-10  7:51 UTC (permalink / raw)
  To: git
In-Reply-To: <20111110074330.GA27925@sigill.intra.peff.net>

When fetching via http, we will try "$URL/info/refs" to get
the list of refs. We may get an unexpected bundle from that
transfer, and we already handle that case. But we should
also check just "$URL" to see if it's a bundle.

Signed-off-by: Jeff King <peff@peff.net>
---
And now we can actually test with apache.

 remote-curl.c          |   19 +++++++++++++++++++
 t/t5552-http-bundle.sh |   36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 0 deletions(-)
 create mode 100755 t/t5552-http-bundle.sh

diff --git a/remote-curl.c b/remote-curl.c
index 84586e0..7734495 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -222,6 +222,25 @@ static int get_refs_from_url(const char *url, struct strbuf *out, int options,
 					     filename, &is_bundle);
 	}
 
+	/* try the straight URL for a bundle, but don't impact the
+	 * error reporting that happens below. */
+	if (http_ret != HTTP_OK && http_ret != HTTP_NOAUTH) {
+		struct strbuf trimmed = STRBUF_INIT;
+		int r;
+
+		strbuf_reset(&buffer);
+
+		strbuf_addstr(&trimmed, url);
+		while (trimmed.len > 0 && trimmed.buf[trimmed.len-1] == '/')
+			strbuf_setlen(&trimmed, trimmed.len - 1);
+
+		r = get_refs_from_url(trimmed.buf, &buffer, 0, filename,
+				      &is_bundle);
+		if (r == HTTP_OK && is_bundle)
+			http_ret = r;
+		strbuf_release(&trimmed);
+	}
+
 	switch (http_ret) {
 	case HTTP_OK:
 		break;
diff --git a/t/t5552-http-bundle.sh b/t/t5552-http-bundle.sh
new file mode 100755
index 0000000..8527403
--- /dev/null
+++ b/t/t5552-http-bundle.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+
+test_description='test fetching from http-accessible bundles'
+. ./test-lib.sh
+
+LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5552'}
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
+
+test_expect_success 'create bundles' '
+	test_commit one &&
+	git bundle create "$HTTPD_DOCUMENT_ROOT_PATH/one.bundle" --all &&
+	test_commit two &&
+	git bundle create "$HTTPD_DOCUMENT_ROOT_PATH/two.bundle" --all ^one
+'
+
+test_expect_success 'clone from bundle' '
+	git clone --bare $HTTPD_URL/one.bundle clone &&
+	echo one >expect &&
+	git --git-dir=clone log -1 --format=%s >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'fetch from bundle' '
+	git --git-dir=clone fetch $HTTPD_URL/two.bundle refs/*:refs/* &&
+	echo two >expect &&
+	git --git-dir=clone log -1 --format=%s >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'cannot clone from partial bundle' '
+	test_must_fail git clone $HTTPD_URL/two.bundle
+'
+
+stop_httpd
+test_done
-- 
1.7.7.2.7.g9f96f

^ permalink raw reply related


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