Git development
 help / color / mirror / Atom feed
* Re: [BUG] git checkout <branch> allowed with uncommitted changes
From: Holger Hellmuth @ 2011-10-14  9:27 UTC (permalink / raw)
  To: Jeff King; +Cc: arQon, git
In-Reply-To: <20111014013830.GA7258@sigill.intra.peff.net>

On 14.10.2011 03:38, Jeff King wrote:
> On Thu, Oct 13, 2011 at 06:56:14PM +0000, arQon wrote:
>
>> I'll give a shot, though I don't know how good it'll be. Off the top of my
>> head, I don't see any good way to explain the inconsistency with LOCAL CHANGES
>> sometimes preventing switches and sometimes not, based on what is to the user
>> an arbitrary set of rules that has nothing to do with the *current state* of
>> the worktree, but rather the state of those files in prior commits.
>
> The rules are fairly straightforward.

They are. But what arQon is getting at is that the normal switchability 
depends on something that is often a game of chance: Did I change a file 
that is different between the two branches? That is only known by the 
user for branches not far removed.

Now the obvious answer is: It doesn't matter because git tells you. At 
the right time to act upon it. But git says "M file" instead of what 
'git status' would say: "#  modified:   file". Is there a reason for 
that? On one hand it should be familiar to svn users, on the other hand 
it is an inconsistency. And personally I always hated those cryptic 
status flags of svn

Another good point arQon made is that the case that you switched with 
forgotten local changes is more common than the case that you switched 
because you made changes in the wrong branch. If that were the case the 
warning that you have local changes should be more visible than that 
small "M file", at best something that looks similar to 'git status' output.

Now what really is more common depends on the individual. If you are a 
beginner or a semi-frequent user, then forgetting local changes is 
probably far more common, wheras most people on this mailing list would 
say its the other way round. It much depends on your commit frequency 
because the more often you commit, the less likely is that you would 
forget local changes.

^ permalink raw reply

* Re: [PATCH] git-svn: Allow certain refs to be ignored
From: Piotr Krukowiecki @ 2011-10-14  9:24 UTC (permalink / raw)
  To: Eric Wong; +Cc: Michael Olson, git, Junio C Hamano
In-Reply-To: <20111010225838.GB3828@dcvr.yhbt.net>

On Tue, Oct 11, 2011 at 12:58 AM, Eric Wong <normalperson@yhbt.net> wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
>> Asking Eric to comment when he has time to do so.
>>
>> I find these pattern matches that are not anchored on either side
>> somewhat disturbing (e.g. --ignore-refs=master would ignore master2)
>> but ignore-paths codepath seems to follow the same pattern, so perhaps it
>> is in line with what git-svn users want. I dunno.
>
> As stated last year, I remember wanting globs instead of regexps, but
> we already made the regexp mistake with ignore-paths, too :(
>
> I don't think it's horrible with regexps, and if git-svn users find it
> useful, it's fine by me.

In my case globs would be too limited. I'm using negative look-ahead
assertions to match only branches/tags for projects that are
interesting to me. With globs it's not possible AFAIK (I would have to
specify all ignored patterns by hand which would work only for known
patterns):
  ignore-paths = ^path/branches/(?!proj1|proj2)


>> Michael Olson <mwolson@gnu.org> writes:
>> > Re-sent by request of Piotr Krukowiecki.  This is against v1.7.4.1,
>> > and I've been using it stably for a while.
>
> Michael: can you please rebase against latest and resend?  Thanks.

I've got following error while using the patch. I don't know it also
happens without the patch...

	M	...
r216099 = 4d16d4890915f4c02ba541956957a4e4b4bed400
(refs/remotes/proj2/proj2-branch9)
Auto packing the repository for optimum performance. You may also
run "git gc" manually. See "git help gc" for more information.
Counting objects: 10284, done.
Compressing objects: 100% (7695/7695), done.
Writing objects: 100% (10284/10284), done.
Total 10284 (delta 5077), reused 0 (delta 0)
fatal: refs/remotes/proj1/trunk: not a valid SHA1
update-ref refs/heads/master refs/remotes/proj1/trunk: command
returned error: 128

$ git log -1
fatal: bad default revision 'HEAD'
$ git log -1 trunk
fatal: ambiguous argument 'trunk': unknown revision or path not in the
working tree.
Use '--' to separate paths from revisions
$ ls .git/refs/remotes/
proj2

The commands I've used:

git svn init --prefix=proj2/ -Rproj2 -s
--ignore-paths='^proj2/branches/(?!proj1|proj2)|^proj2/tags/(?!proj1|proj2)|^proj2/(?!trunk|branches|tags)'
--ignore-refs='^refs/remotes/proj2/(?!proj1|proj2|trunk|tag)|^refs/remotes/proj2/tags/(?!proj1|proj2)'
http://url/svn/repos/proj2
git svn init --prefix=proj1/ -Rproj1 -s
--ignore-paths='^proj1/branches/(?!proj1|proj2)|^proj1/tags/(?!proj1|proj2)|^proj2/(?!trunk|branches|tags)'
--ignore-refs='^refs/remotes/proj1/(?!proj1|proj2|trunk|tag)|^refs/remotes/proj1/tags/(?!proj1|proj2)'
http://url/svn/repos/proj1
git svn fetch proj2 && git svn fetch proj1

The error happened while fetching proj2. I wonder why it tried to set
master to proj1?
Is my command ok? I want to have both proj1 and proj2 under one git repository.


-- 
Piotr Krukowiecki

^ permalink raw reply

* Re: defined behaviour for multiple urls for a remote
From: Kirill Likhodedov @ 2011-10-14  9:36 UTC (permalink / raw)
  To: git
In-Reply-To: <CAMK1S_jZJuqC6_-eVT7LJFh+DEphbsypS6f4nRb6Qc4-xBa_wQ@mail.gmail.com>

Sitaram Chamarty <sitaramc <at> gmail.com> writes:

> What's the defined behaviour if I do this:
> 
> [remote "both"]
> 	url = https://code.google.com/p/gitolite/
>         url = git <at> github.com:sitaramc/gitolite.git
> 
> I know what I'm seeing (a fetch only goes to the first URL, and does a
> HEAD->FETCH_HEAD because I didn't provide a refspec line, while a push
> seems to push all to both), but I was curious what the official
> position is, because I couldn't find it in the docs.

Please see the message from Linus about that: http://marc.info/?l=git&m=116231242118202&w=2

You also may check how Git understands your remotes by running
  git remote -v
It will show, where it is going to fetch from and push to.

I agree though, that documentation should be updated.

^ permalink raw reply

* Re: [BUG] git checkout <branch> allowed with uncommitted changes
From: Victor Engmark @ 2011-10-14  9:54 UTC (permalink / raw)
  To: Holger Hellmuth; +Cc: Jeff King, arQon, git
In-Reply-To: <4E980093.6040704@ira.uka.de>

On Fri, Oct 14, 2011 at 11:27:47AM +0200, Holger Hellmuth wrote:
> On 14.10.2011 03:38, Jeff King wrote:
> >On Thu, Oct 13, 2011 at 06:56:14PM +0000, arQon wrote:
> >
> >>I'll give a shot, though I don't know how good it'll be. Off the top of my
> >>head, I don't see any good way to explain the inconsistency with LOCAL CHANGES
> >>sometimes preventing switches and sometimes not, based on what is to the user
> >>an arbitrary set of rules that has nothing to do with the *current state* of
> >>the worktree, but rather the state of those files in prior commits.
> >
> >The rules are fairly straightforward.
> 
> They are. But what arQon is getting at is that the normal
> switchability depends on something that is often a game of chance:
> Did I change a file that is different between the two branches? That
> is only known by the user for branches not far removed.
> 
> Now the obvious answer is: It doesn't matter because git tells you.
> At the right time to act upon it. But git says "M file" instead of
> what 'git status' would say: "#  modified:   file". Is there a
> reason for that? On one hand it should be familiar to svn users, on
> the other hand it is an inconsistency. And personally I always hated
> those cryptic status flags of svn
> 
> Another good point arQon made is that the case that you switched
> with forgotten local changes is more common than the case that you
> switched because you made changes in the wrong branch. If that were
> the case the warning that you have local changes should be more
> visible than that small "M file", at best something that looks
> similar to 'git status' output.

Very good point. How about by default just running `git status` after a
successful checkout, and only printing the result if there are any
changes? That way:
1) If no changes are pending, nothing is displayed.
2) The user sees a *familiar* style output if anything changed.
3) If there's an alias for "status", it would be used.

Example:

$ mkdir /tmp/test
$ cd /tmp/test
$ git init
Initialized empty Git repository in /tmp/test/.git/
$ echo foo > foo
$ echo bar > bar
$ git add foo bar
$ git commit -m "Initial commit"
[master (root-commit) 55246c6] Initial commit
 2 files changed, 2 insertions(+), 0 deletions(-)
 create mode 100644 bar
 create mode 100644 foo
$ echo foobar > bar
$ git branch --track test
Branch test set up to track local branch master.
$ git checkout test
M   bar
Switched to branch 'test'

After `git checkout test`, we should instead see:
# On branch test
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)
#
#   modified:   bar
#
no changes added to commit (use "git add" and/or "git commit -a")

2c,
V

-- 
terreActive AG
Kasinostrasse 30
CH-5001 Aarau
Tel: +41 62 834 00 55
Fax: +41 62 823 93 56
www.terreactive.ch

Wir sichern Ihren Erfolg - seit 15 Jahren

^ permalink raw reply

* Git shouldn't allow to push a new branch called HEAD
From: Daniele Segato @ 2011-10-14 11:31 UTC (permalink / raw)
  To: Git Mailing List

Hi all,


following from a discussion in IRC freenode #git between me, sitaram an
shruggar


step to reproduce:

$ mkdir /tmp/gitbug
$ cd /tmp/gitbug/

$ # create a fake remote repo
$ git init --bare remote.git

$ # clone it with the user that will generate the bug
$ git clone remote.git buggenerator
$ cd buggenerator/
$ touch whatever
$ git add .
$ git commit -m "first commit"
$ git push origin master 

$ # now clone the same repo the other guy is the "victim" of this issue
$ cd ..
$ git clone remote.git victim

$ # time to create the remote HEAD branch
$ cd buggenerator/
$ git push origin HEAD:HEAD

$ # the remote refs has been created!
$ git ls-remote

$ # another commit
$ echo 'any change' >> whatever 
$ git commit -a -m "some change"
$ git push origin master 

$ # the refs/heads/HEAD is still where it was
$ git ls-remote

$ # now from the victim perspective
$ cd ../victim/

$ # every time executing a fetch he will get a force update
$ # or maybe even an error, seen it my real repo, don't know how
$ # to reproduce
$ git fetch 
$ git fetch 
$ git ls-remote
$ git fetch 
$ git ls-remote
$ git branch -a



full console log:

mastro@mastroc3 ~  $ mkdir /tmp/gitbug
mastro@mastroc3 ~  $ cd /tmp/gitbug/
mastro@mastroc3 /tmp/gitbug  $ git init --bare remote.git
Initialized empty Git repository in /tmp/gitbug/remote.git/
mastro@mastroc3 /tmp/gitbug  $ git clone remote.git buggenerator
Cloning into buggenerator...
done.
warning: You appear to have cloned an empty repository.
mastro@mastroc3 /tmp/gitbug  $ cd buggenerator/
mastro@mastroc3 /tmp/gitbug/buggenerator (master #) $ touch whatever
mastro@mastroc3 /tmp/gitbug/buggenerator (master #) $ git add .
mastro@mastroc3 /tmp/gitbug/buggenerator (master #) $ git commit -m
"first commit"
[master (root-commit) 11d0a12] first commit
 0 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 whatever
mastro@mastroc3 /tmp/gitbug/buggenerator (master) $ git push origin
master 
Counting objects: 3, done.
Writing objects: 100% (3/3), 213 bytes, done.
Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
To /tmp/gitbug/remote.git
 * [new branch]      master -> master
mastro@mastroc3 /tmp/gitbug/buggenerator (master) $ cd ..
mastro@mastroc3 /tmp/gitbug  $ git clone remote.git victim
Cloning into victim...
done.
mastro@mastroc3 /tmp/gitbug  $ cd buggenerator/

# now creating the HEAD remote branch

mastro@mastroc3 /tmp/gitbug/buggenerator (master) $ git push origin
HEAD:HEAD
Total 0 (delta 0), reused 0 (delta 0)
To /tmp/gitbug/remote.git
 * [new branch]      HEAD -> HEAD
mastro@mastroc3 /tmp/gitbug/buggenerator (master) $ git ls-remote
From /tmp/gitbug/remote.git
11d0a122125e50e78c7aa4aa81a3d6090dba648e	HEAD
11d0a122125e50e78c7aa4aa81a3d6090dba648e	refs/heads/HEAD <-----
shouldn't be there!
11d0a122125e50e78c7aa4aa81a3d6090dba648e	refs/heads/master
mastro@mastroc3 /tmp/gitbug/buggenerator (master) $ echo 'any change' >>
whatever 
mastro@mastroc3 /tmp/gitbug/buggenerator (master *) $ git commit -a -m
"some change"
[master 77852ef] some change
 1 files changed, 1 insertions(+), 0 deletions(-)
mastro@mastroc3 /tmp/gitbug/buggenerator (master) $ git push origin
master 
Counting objects: 5, done.
Writing objects: 100% (3/3), 253 bytes, done.
Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
To /tmp/gitbug/remote.git
   11d0a12..77852ef  master -> master
mastro@mastroc3 /tmp/gitbug/buggenerator (master) $ git ls-remote
From /tmp/gitbug/remote.git
77852effa972187d60d4c75145198991f1c0f868	HEAD
11d0a122125e50e78c7aa4aa81a3d6090dba648e	refs/heads/HEAD
77852effa972187d60d4c75145198991f1c0f868	refs/heads/master
mastro@mastroc3 /tmp/gitbug/buggenerator (master) $ cd ../victim/
mastro@mastroc3 /tmp/gitbug/victim (master) $ git fetch 
remote: Counting objects: 5, done.
remote: Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
From /tmp/gitbug/remote
   11d0a12..77852ef  master     -> origin/master
mastro@mastroc3 /tmp/gitbug/victim (master) $ git fetch 
From /tmp/gitbug/remote
 + 77852ef...11d0a12 HEAD       -> origin/HEAD  (forced update)
mastro@mastroc3 /tmp/gitbug/victim (master) $ git fetch 
From /tmp/gitbug/remote
   11d0a12..77852ef  master     -> origin/master
mastro@mastroc3 /tmp/gitbug/victim (master) $ git ls-remote
From /tmp/gitbug/remote.git
77852effa972187d60d4c75145198991f1c0f868	HEAD
11d0a122125e50e78c7aa4aa81a3d6090dba648e	refs/heads/HEAD
77852effa972187d60d4c75145198991f1c0f868	refs/heads/master
mastro@mastroc3 /tmp/gitbug/victim (master) $ git fetch 
From /tmp/gitbug/remote
 + 77852ef...11d0a12 HEAD       -> origin/HEAD  (forced update)
mastro@mastroc3 /tmp/gitbug/victim (master) $ git ls-remote
From /tmp/gitbug/remote.git
77852effa972187d60d4c75145198991f1c0f868	HEAD
11d0a122125e50e78c7aa4aa81a3d6090dba648e	refs/heads/HEAD
77852effa972187d60d4c75145198991f1c0f868	refs/heads/master
mastro@mastroc3 /tmp/gitbug/victim (master) $ git branch -a
* master
  remotes/origin/HEAD -> origin/master
  remotes/origin/master




this can be fixed with:

git push --delete origin HEAD
(or git push origin :HEAD)

then
git remote prune origin



But I think that git shouldn't allow the remote HEAD reference to be
created in the first place

regards,
Daniele

^ permalink raw reply

* Re: Git shouldn't allow to push a new branch called HEAD
From: Daniele Segato @ 2011-10-14 11:35 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <1318591877.2938.20.camel@mastroc3.mobc3.local>

On Fri, 2011-10-14 at 13:31 +0200, Daniele Segato wrote:
> Hi all,
> 
> 
> following from a discussion in IRC freenode #git between me, sitaram an
> shruggar
> 
> 
> step to reproduce:
> 
> $ mkdir /tmp/gitbug
> $ cd /tmp/gitbug/
> 
> $ # create a fake remote repo
> $ git init --bare remote.git
> 
> $ # clone it with the user that will generate the bug
> $ git clone remote.git buggenerator
> $ cd buggenerator/
> $ touch whatever
> $ git add .
> $ git commit -m "first commit"
> $ git push origin master 
> 
> $ # now clone the same repo the other guy is the "victim" of this issue
> $ cd ..
> $ git clone remote.git victim
> 
> $ # time to create the remote HEAD branch
> $ cd buggenerator/
> $ git push origin HEAD:HEAD
> 
> $ # the remote refs has been created!
> $ git ls-remote
> 
> $ # another commit
> $ echo 'any change' >> whatever 
> $ git commit -a -m "some change"
> $ git push origin master 
> 
> $ # the refs/heads/HEAD is still where it was
> $ git ls-remote
> 
> $ # now from the victim perspective
> $ cd ../victim/
> 
> $ # every time executing a fetch he will get a force update
> $ # or maybe even an error, seen it my real repo, don't know how
> $ # to reproduce
> $ git fetch 
> $ git fetch 
> $ git ls-remote
> $ git fetch 
> $ git ls-remote
> $ git branch -a

This should also help understanding what happen in the "victim" local
repo at every fetch:

mastro@mastroc3 /tmp/gitbug/victim (master) $ git br -av
* master                11d0a12 [behind 1] first commit
  remotes/origin/HEAD   -> origin/master
  remotes/origin/master 77852ef some change
mastro@mastroc3 /tmp/gitbug/victim (master) $ git fetch 
From /tmp/gitbug/remote
 + 77852ef...11d0a12 HEAD       -> origin/HEAD  (forced update)
mastro@mastroc3 /tmp/gitbug/victim (master) $ git br -av
* master                11d0a12 first commit
  remotes/origin/HEAD   -> origin/master
  remotes/origin/master 11d0a12 first commit


regards,
Daniele

^ permalink raw reply

* [PATCH] t7800: avoid arithmetic expansion notation
From: Michael J Gruber @ 2011-10-14 12:15 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Sitaram Chamarty

ba959de (git-difftool: allow skipping file by typing 'n' at prompt, 2011-10-08)
introduced shell code like

$((foo; bar) | baz)

which some shells (e.g. bash, dash) interpret as an unfinished arithmetic
evaluation $(( expr )).

Write this as

$({foo; bar; } | baz)

which is unambiguous and also avoids an unnecessary subshell.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 t/t7800-difftool.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 7fc2b3a..76a8b70 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -289,7 +289,7 @@ test_expect_success PERL 'setup with 2 files different' '
 '
 
 test_expect_success PERL 'say no to the first file' '
-	diff=$((echo n; echo) | git difftool -x cat branch) &&
+	diff=$({ echo n; echo; } | git difftool -x cat branch) &&
 
 	echo "$diff" | stdin_contains m2 &&
 	echo "$diff" | stdin_contains br2 &&
@@ -298,7 +298,7 @@ test_expect_success PERL 'say no to the first file' '
 '
 
 test_expect_success PERL 'say no to the second file' '
-	diff=$((echo; echo n) | git difftool -x cat branch) &&
+	diff=$({ echo; echo n; } | git difftool -x cat branch) &&
 
 	echo "$diff" | stdin_contains master &&
 	echo "$diff" | stdin_contains branch &&
-- 
1.7.7.338.g0156b

^ permalink raw reply related

* Re: [PATCH 2/9] completion: optimize refs completion
From: SZEDER Gábor @ 2011-10-14 12:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Shawn O. Pearce, Jonathan Nieder
In-Reply-To: <1318085683-29830-3-git-send-email-szeder@ira.uka.de>

On Sat, Oct 08, 2011 at 04:54:36PM +0200, SZEDER Gábor wrote:
> So, add a specialized variant of __gitcomp() that only deals with
> possible completion words separated by a newline 

> @@ -2635,7 +2659,7 @@ _git ()
>  			"
>  			;;
>  		*)     __git_compute_porcelain_commands
> -		       __gitcomp "$__git_porcelain_commands $(__git_aliases)" ;;
> +		       __gitcomp_nl "$__git_porcelain_commands $(__git_aliases)" ;;
>  		esac
>  		return
>  	fi

Oops, this last hunk is wrong.

I made the thinko that $__git_porcelain_commands is NL-separated and
the output of __git_aliases() is NL-separated, so we can pass the two
together to the new __gitcomp_nl() function.  But of course not,
because the SP between the two joins the last command and the first
alias.

I will resend in the evening with this hunk removed and the commit
message updated.


Gábor

^ permalink raw reply

* [BUG] send-email: alias expansion broken
From: Michael J Gruber @ 2011-10-14 12:29 UTC (permalink / raw)
  To: Git Mailing List, Cord Seele

cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)

broke the expansion of aliases for me:

./git-send-email --cc=junio  --dry-run
0001-t7800-avoid-arithmetic-expansion-notation.patch
0001-t7800-avoid-arithmetic-expansion-notation.patch
Who should the emails appear to be from? [Michael J Gruber
<git@drmicha.warpmail.net>]
Emails will be sent from: Michael J Gruber <git@drmicha.warpmail.net>
Dry-OK. Log says:
Sendmail: /home/mjg/bin/msmtp-fastmail-git -i git@vger.kernel.org junio
git@drmicha.warpmail.net
From: Michael J Gruber <git@drmicha.warpmail.net>
To: git@vger.kernel.org
Cc: junio
...

Happens with both "--cc junio" and "--cc=junio".

Reverting cec5dae brings my aliases back. Relevant config:

git config --get-regexp sendemail.alias\*
sendemail.aliasesfile /home/mjg/git/gitauthors
sendemail.aliasfiletype mutt

Can I please have alias expansion back?

No, I don't know what cec5dae tries to achieve, and I lack the perl fu
to fix it.

Michael

^ permalink raw reply

* Re: [RFC/PATCH 1/2] bundle: allowing to read from an unseekable fd
From: Phil Hord @ 2011-10-14 12:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, git
In-Reply-To: <7vpqi034l0.fsf@alter.siamese.dyndns.org>

On Thu, Oct 13, 2011 at 6:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
> The current code opens a given file with fopen(), reads it until the end
> of the header and runs ftell(), and reopens the same file with open() and
> seeks to skip the header. This structure makes it hard to retarget the
> code to read from input that is not seekable, such as a network socket.
>
> This patch by itself does not reach that goal yet, but I think it is a
> right step in that direction.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>
>  * It would be nice if we can avoid byte-by-byte reading from the file
>   descriptor by over-reading into the strbuf and pass the remainder to
>   the caller of read_bundle_header(), but I suspect that it would require
>   us to carry the "here is the remainder from the previous read" buffer
>   around throughout the transport layer. Parsing of the header wouldn't
>   be performance critical compared to the computation cost of actually
>   reading the rest of the bundle, hopefully, so...
>
>  bundle.c |   99 ++++++++++++++++++++++++++++++++++++++++----------------------
>  1 files changed, 64 insertions(+), 35 deletions(-)
>
> diff --git a/bundle.c b/bundle.c
> index f48fd7d..3aa715c 100644
> --- a/bundle.c
> +++ b/bundle.c
> @@ -23,49 +23,78 @@ static void add_to_ref_list(const unsigned char *sha1, const char *name,
>        list->nr++;
>  }
>
> -/* returns an fd */
> +/* Eventually this should go to strbuf.[ch] */
> +static int strbuf_readline_fd(struct strbuf *sb, int fd)

A size limiter would be useful here.  Since it's readline, maybe the
limit can even be hardcoded.

Without a limit, calling this with something stupid like "/dev/zero"
will consume all memory and never return.


Phil

^ permalink raw reply

* Re: [PATCH 2/2] bundle: add parse_bundle_header() helper function
From: Phil Hord @ 2011-10-14 12:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Shawn O. Pearce, Phil Hord
In-Reply-To: <7vliso34gc.fsf@alter.siamese.dyndns.org>

On Thu, Oct 13, 2011 at 6:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Move most of the code from read_bundle_header() to parse_bundle_header()
> that takes a file descriptor that is already opened for reading, and make
> the former responsible only for opening the file and noticing errors.
...

>  * It generally is a bad practice to base a non-RFC patch on an RFC one,
>   but in any case here is how I would do the is_bundle() helper.

I didn't label it RFC, but I did pose a question in the message
section.  Is there a rule for what marks something as RFC (if not the
RFC label in the subject line)?

Your implementation looks fine to me.

Phil

^ permalink raw reply

* Re: [PATCH] pack-objects: protect against disappearing packs
From: Jeff King @ 2011-10-14 13:02 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git, git-dev, Shawn O. Pearce
In-Reply-To: <alpine.LFD.2.02.1110132234210.17040@xanadu.home>

On Thu, Oct 13, 2011 at 10:42:28PM -0400, Nicolas Pitre wrote:

> > ---
> 
> you should put your SOB above that line I would think.

Thanks. I cheated and wrote my "---" cover letter in the commit message
locally, knowing that it would get included by format-patch but stripped
by am on Junio's end.  Which does work, except that "format-patch -s"
puts the SOB in the wrong place. :)

> Acked-by: Nicolas Pitre <nico@fluxnic.net>

Thanks for reviewing.

> > We're seeing this at GitHub because we prune pretty
> > aggressively. We let pushes go into individual repositories,
> > but then we kick off a job to move the resulting objects
> > into the repository's "network" repo, which is basically a
> > big alternates repository for related repos.
> 
> While this patch certainly has value, it doesn't provide 100% 
> reliability for that use case.  Maybe the github infrastructure should 
> simply skip any auto-repack on push if some other object maintenance 
> operation is ongoing, possibly via the pre-auto-gc hook.

I'm not sure I understand the problem.  We already serialize the
re-packing jobs in a queue, so you won't have two repacks going at once.
The problematic pack-objects is the one started by upload-pack when
somebody fetches. Or do you mean turning off receive.autogc? I'd have to
check if we do that, but we definitely should; it's useless to us
(though it would be unlikely to trigger anyway, because we are manually
repacking so frequently).

-Peff

^ permalink raw reply

* Re: [PATCH] pack-objects: protect against disappearing packs
From: Jeff King @ 2011-10-14 13:07 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, git-dev, Shawn O. Pearce, Nicolas Pitre
In-Reply-To: <4E97DF63.104@viscovery.net>

On Fri, Oct 14, 2011 at 09:06:11AM +0200, Johannes Sixt wrote:

> Am 10/14/2011 3:23, schrieb Jeff King:
> > In practice, however, adding this check still has value, for
> > three reasons.
> > 
> >   1. If you have a reasonable number of packs and/or a
> >      reasonable file descriptor limit, you can keep all of
> >      your packs open simultaneously. If this is the case,
> >      then the race is impossible to trigger.
> 
> On Windows, we cannot remove files that are open. If I understand
> correctly, this patch keeps more files open for a longer time. Is there
> any chance that packfiles remain now open until an unlink() call?
> 
> I am not worried about parallel processes (we already have a problem
> there), but that this can now happen within a single process, i.e., that a
> single git-repack -a -d -f would now try to unlink a pack file that it
> opened itself and did not close timely.
> 
> I'll test your patch later this weekend to see whether the test suite
> finds something. But perhaps you know the answer already?

With two parallel processes, this will definitely increase the
likelihood of a deleted file being open. That is the point. :)

Within a single process, I don't think so. This change impacts only
pack-objects, which always runs as a separate process, and never deletes
packs itself. The most likely problematic code path would be "git
repack -d", but it waits for pack-objects to complete successfully
before removing any packs.

-Peff

^ permalink raw reply

* Re: [PATCH] daemon: return "access denied" if a service is not allowed
From: Jeff King @ 2011-10-14 13:10 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyen Thai Ngoc Duy, git, Ilari Liusvaara, Johannes Sixt,
	Jonathan Nieder
In-Reply-To: <7vvcrs181e.fsf@alter.siamese.dyndns.org>

On Thu, Oct 13, 2011 at 10:01:01PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > So if we want to do anything, I would think it would be a hook. Except
> > that we may or may not have a repo, so it would not be a hook in
> > $GIT_DIR/hooks, but rather some script to be run passed on the command
> > line, like:
> >
> >   git daemon --informative-errors=/path/to/hook
> 
> I don't think it is necessarily good to have such a variation across
> hosting sites. Your "something like this" patch looked like it was giving
> a reasonable level of detail, IMO.

Yeah. With arbitrary messages, the client has no way of programatically
deciphering which message is which, so localization becomes impossible.
HTTP solved this by having a response code _and_ still allowing content
for custom pages.  That kind of works, though most of the time I find
things like custom 404 pages to just be junk.

Let's start with my original patch (which I'll clean and repost). And if
somebody really wants to push towards customized messages, that is easy
enough to do on top later.

-Peff

^ permalink raw reply

* Re: [PATCH 0/6] http-auth-early
From: Jeff King @ 2011-10-14 13:19 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <cover.1318577792.git.git@drmicha.warpmail.net>

On Fri, Oct 14, 2011 at 09:40:34AM +0200, Michael J Gruber wrote:

> Here are the early parts of Jeff's http-auth-keyring series.
> It contains only parts which are not using the credential API (which
> is still under discussion), so that this can go in (and help users)
> and alleviates the pressure on the credential discussion:
> 
> Early bits with cleanups to http.c.
> Cherry-picked bit for improved prompts ("Username for ..." etc.)
> Cherry-pickes bit for using configured pushurls.
> 
> I tried to pick/resolve in a way which should help rebasing Jeff's series
> on top of this.

Thanks for working on this. One of my intended tasks for today is to
rebase my series, so it is nice to wake up to half of the work done. :)

> Jeff King (5):
>   url: decode buffers that are not NUL-terminated
>   improve httpd auth tests
>   remote-curl: don't retry auth failures with dumb protocol
>   http: retry authentication failures for all http requests
>   http_init: accept separate URL parameter
> 
> Michael J Gruber (1):
>   http: use hostname in credential description

Your changes all look right. The naming of git_getpass_one in the
cherry-picked commit is a little odd without the rest of the series as
context. I would maybe have called it "git_getpass_with_description" or
something.

-Peff

^ permalink raw reply

* Re: [PATCH 0/6] http-auth-early
From: Michael J Gruber @ 2011-10-14 13:24 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20111014131932.GE7808@sigill.intra.peff.net>

Jeff King venit, vidit, dixit 14.10.2011 15:19:
> On Fri, Oct 14, 2011 at 09:40:34AM +0200, Michael J Gruber wrote:
> 
>> Here are the early parts of Jeff's http-auth-keyring series.
>> It contains only parts which are not using the credential API (which
>> is still under discussion), so that this can go in (and help users)
>> and alleviates the pressure on the credential discussion:
>>
>> Early bits with cleanups to http.c.
>> Cherry-picked bit for improved prompts ("Username for ..." etc.)
>> Cherry-pickes bit for using configured pushurls.
>>
>> I tried to pick/resolve in a way which should help rebasing Jeff's series
>> on top of this.
> 
> Thanks for working on this. One of my intended tasks for today is to
> rebase my series, so it is nice to wake up to half of the work done. :)

Good morning :)

>> Jeff King (5):
>>   url: decode buffers that are not NUL-terminated
>>   improve httpd auth tests
>>   remote-curl: don't retry auth failures with dumb protocol
>>   http: retry authentication failures for all http requests
>>   http_init: accept separate URL parameter
>>
>> Michael J Gruber (1):
>>   http: use hostname in credential description
> 
> Your changes all look right. The naming of git_getpass_one in the
> cherry-picked commit is a little odd without the rest of the series as
> context. I would maybe have called it "git_getpass_with_description" or
> something.

git_getpass_my_life_will_be_short_and_ended_by_credentials

I don't care. In fact, I wasn't sure whether I should I even change the
author on this one. It's not a straight resolution and does involve
choices, but the meat is from your series.

Michael

^ permalink raw reply

* Bug? url.insteadOf overwrites remote.pushUrl
From: Kirill Likhodedov @ 2011-10-14 13:55 UTC (permalink / raw)
  To: git


I've found that defining url.<base>.insteadOf overrides explicit remote.<name>.pushUrl.
On the other hand, pushInsteadOf doesn't override explicit pushUrl.
Is it a bug?

# cat .git/config
[remote "origin"]
	fetch = +refs/heads/*:refs/remotes/origin/*
	url = github.com/klikh/Test.git 
	pushUrl = jetbrains.com/klikh/Test.git 
[url "http://"]
  insteadOf=jet

# git remote -v
origin	github.com/klikh/Test.git (fetch)
origin	http://brains.com/klikh/Test.git (push)


See also http://kerneltrap.org/mailarchive/git/2009/9/7/11264/thread - patch introducing pushInsteadOf & discussion 

----------------------------------
Kirill Likhodedov
JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"

^ permalink raw reply

* Re: defined behaviour for multiple urls for a remote
From: Sitaram Chamarty @ 2011-10-14 14:01 UTC (permalink / raw)
  To: Kirill Likhodedov; +Cc: git
In-Reply-To: <loom.20111014T113208-660@post.gmane.org>

On Fri, Oct 14, 2011 at 3:06 PM, Kirill Likhodedov
<Kirill.Likhodedov@jetbrains.com> wrote:
> Sitaram Chamarty <sitaramc <at> gmail.com> writes:
>
>> What's the defined behaviour if I do this:
>>
>> [remote "both"]
>>       url = https://code.google.com/p/gitolite/
>>         url = git <at> github.com:sitaramc/gitolite.git
>>
>> I know what I'm seeing (a fetch only goes to the first URL, and does a
>> HEAD->FETCH_HEAD because I didn't provide a refspec line, while a push
>> seems to push all to both), but I was curious what the official
>> position is, because I couldn't find it in the docs.
>
> Please see the message from Linus about that: http://marc.info/?l=git&m=116231242118202&w=2

cool; thanks!

> You also may check how Git understands your remotes by running
>  git remote -v

Aah that's very clear!

> It will show, where it is going to fetch from and push to.
>
> I agree though, that documentation should be updated.

well if it never came up once in the 5 years since Linus wrote that email... :-)

^ permalink raw reply

* [PATCH] fix alias expansion with new Git::config_path()
From: Cord Seele @ 2011-10-14 14:25 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List, Junio C Hamano, Jakub Narebski
In-Reply-To: <4E982B27.8050807@drmicha.warpmail.net>

On Fri 14 Oct 2011 14:29:27 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:

> cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)
> 
> broke the expansion of aliases for me:
> 
> ./git-send-email --cc=junio  --dry-run
> 0001-t7800-avoid-arithmetic-expansion-notation.patch
> 0001-t7800-avoid-arithmetic-expansion-notation.patch
> Who should the emails appear to be from? [Michael J Gruber
> <git@drmicha.warpmail.net>]
> Emails will be sent from: Michael J Gruber <git@drmicha.warpmail.net>
> Dry-OK. Log says:
> Sendmail: /home/mjg/bin/msmtp-fastmail-git -i git@vger.kernel.org junio
> git@drmicha.warpmail.net
> From: Michael J Gruber <git@drmicha.warpmail.net>
> To: git@vger.kernel.org
> Cc: junio
> ...
> 
> Happens with both "--cc junio" and "--cc=junio".
> 
> Reverting cec5dae brings my aliases back. Relevant config:
> 
> git config --get-regexp sendemail.alias\*
> sendemail.aliasesfile /home/mjg/git/gitauthors
> sendemail.aliasfiletype mutt
> 
> Can I please have alias expansion back?

The following patch fixes it for me, please give it a try.

Since this fix is simply copy&pasting some code from the config_settings path
someone with better perl understanding might wnat to refactor it
(Junio/Jacob)?

-- Cord


Signed-off-by: Cord Seele <cowose@gmail.com>
---
 git-send-email.perl |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 91607c5..6885dfa 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -337,8 +337,16 @@ sub read_config {
 	}
 
 	foreach my $setting (keys %config_path_settings) {
-		my $target = $config_path_settings{$setting}->[0];
-		$$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
+		my $target = $config_path_settings{$setting};
+		if (ref($target) eq "ARRAY") {
+			unless (@$target) {
+				my @values = Git::config_path(@repo, "$prefix.$setting");
+				@$target = @values if (@values && defined $values[0]);
+			}
+		}
+		else {
+			$$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
+		}
 	}
 
 	foreach my $setting (keys %config_settings) {
-- 
1.7.6.4

^ permalink raw reply related

* Re: [PATCH] fix alias expansion with new Git::config_path()
From: Michael J Gruber @ 2011-10-14 14:30 UTC (permalink / raw)
  To: Git Mailing List, Junio C Hamano, Jakub Narebski
In-Reply-To: <20111014142557.GB13680@laptop>

Cord Seele venit, vidit, dixit 14.10.2011 16:25:
> On Fri 14 Oct 2011 14:29:27 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:
> 
>> cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)
>>
>> broke the expansion of aliases for me:
>>
>> ./git-send-email --cc=junio  --dry-run
>> 0001-t7800-avoid-arithmetic-expansion-notation.patch
>> 0001-t7800-avoid-arithmetic-expansion-notation.patch
>> Who should the emails appear to be from? [Michael J Gruber
>> <git@drmicha.warpmail.net>]
>> Emails will be sent from: Michael J Gruber <git@drmicha.warpmail.net>
>> Dry-OK. Log says:
>> Sendmail: /home/mjg/bin/msmtp-fastmail-git -i git@vger.kernel.org junio
>> git@drmicha.warpmail.net
>> From: Michael J Gruber <git@drmicha.warpmail.net>
>> To: git@vger.kernel.org
>> Cc: junio
>> ...
>>
>> Happens with both "--cc junio" and "--cc=junio".
>>
>> Reverting cec5dae brings my aliases back. Relevant config:
>>
>> git config --get-regexp sendemail.alias\*
>> sendemail.aliasesfile /home/mjg/git/gitauthors
>> sendemail.aliasfiletype mutt
>>
>> Can I please have alias expansion back?
> 
> The following patch fixes it for me, please give it a try.
> 
> Since this fix is simply copy&pasting some code from the config_settings path
> someone with better perl understanding might wnat to refactor it
> (Junio/Jacob)?
> 
> -- Cord
> 
> 
> Signed-off-by: Cord Seele <cowose@gmail.com>

Tested-by: Michael J Gruber <git@drmicha.warpmail.net>


Thanks. (Though I'm still wondering what this is about overall.)


> ---
>  git-send-email.perl |   12 ++++++++++--
>  1 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 91607c5..6885dfa 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -337,8 +337,16 @@ sub read_config {
>  	}
>  
>  	foreach my $setting (keys %config_path_settings) {
> -		my $target = $config_path_settings{$setting}->[0];
> -		$$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
> +		my $target = $config_path_settings{$setting};
> +		if (ref($target) eq "ARRAY") {
> +			unless (@$target) {
> +				my @values = Git::config_path(@repo, "$prefix.$setting");
> +				@$target = @values if (@values && defined $values[0]);
> +			}
> +		}
> +		else {
> +			$$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
> +		}
>  	}
>  
>  	foreach my $setting (keys %config_settings) {

^ permalink raw reply

* Re: [PATCH] pack-objects: protect against disappearing packs
From: Johannes Sixt @ 2011-10-14 14:35 UTC (permalink / raw)
  To: Jeff King; +Cc: git, git-dev, Shawn O. Pearce, Nicolas Pitre
In-Reply-To: <20111014130703.GB7808@sigill.intra.peff.net>

Am 10/14/2011 15:07, schrieb Jeff King:
> Within a single process, I don't think so. This change impacts only
> pack-objects, which always runs as a separate process, and never deletes
> packs itself. The most likely problematic code path would be "git
> repack -d", but it waits for pack-objects to complete successfully
> before removing any packs.

Thanks. The test suite didn't find any issues on Windows.

-- Hannes

^ permalink raw reply

* Re: [PATCH] fix alias expansion with new Git::config_path()
From: Cord Seele @ 2011-10-14 14:42 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List, Junio C Hamano, Jakub Narebski
In-Reply-To: <4E984781.6050601@drmicha.warpmail.net>

On Fri 14 Oct 2011 16:30:25 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:

> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>

Great.

> Thanks. (Though I'm still wondering what this is about overall.)

to make '~/' work in sendemail.aliasesfile

-- Cord

^ permalink raw reply

* Re: [PATCH] fix alias expansion with new Git::config_path()
From: Junio C Hamano @ 2011-10-14 15:05 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List, Jakub Narebski
In-Reply-To: <20111014144203.GC13680@laptop>

Cord Seele <cowose@googlemail.com> writes:

> On Fri 14 Oct 2011 16:30:25 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:
>
>> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
>
> Great.
>
>> Thanks. (Though I'm still wondering what this is about overall.)
>
> to make '~/' work in sendemail.aliasesfile

That is not an explanation.

^ permalink raw reply

* Re: Bug? url.insteadOf overwrites remote.pushUrl
From: Michael J Gruber @ 2011-10-14 15:35 UTC (permalink / raw)
  To: Kirill Likhodedov; +Cc: git
In-Reply-To: <CAB6D58F-A3C9-4532-A9CC-10E43CD34E4E@jetbrains.com>

Kirill Likhodedov venit, vidit, dixit 14.10.2011 15:55:
> 
> I've found that defining url.<base>.insteadOf overrides explicit
> remote.<name>.pushUrl.

It doesn't really override it. It is applied to it, i.e. transforms it.

> On the other hand, pushInsteadOf doesn't
> override explicit pushUrl. Is it a bug?

That is as described in the thread (thanks for linking to it).

> # cat .git/config [remote "origin"] fetch =
> +refs/heads/*:refs/remotes/origin/* url = github.com/klikh/Test.git 
> pushUrl = jetbrains.com/klikh/Test.git [url "http://"] insteadOf=jet
> 
> # git remote -v origin	github.com/klikh/Test.git (fetch) origin
> http://brains.com/klikh/Test.git (push)

The idea of "pushInsteadOf" was that, instead of having to define url
and pushurl separately, you can use different rules, say

github -> git://github.com (fetch)
github -> https://username@github.com (push)
github/ -> git@github.com: (push)

used with "url = github/otheruser/repo.git" alone, without pushurl.

pushurl predates pushinsteadof, and when the latter was introduced, one
could have argued for or against "insteadof" being applied to pushurls.
But that was necessary before, and existing behavior at the time when
pushinsteadof was introduced. So, I don't see a bug, nor anything we
could change now, though arguably most people use either pushinstead of
or pushurl, but not both.

Cheers,
Michael

^ permalink raw reply

* [BUG] remote-curl.c: honor pushurl
From: Michael Schubert @ 2011-10-14 15:37 UTC (permalink / raw)
  To: git; +Cc: rctay89

When doing a push (fetch, ..) over http(s), git calls git-remote-http to
communicate with the server.

	git-remote-http <remote> [<url>]

Git correctly honors a configured pushurl and passes it to git-remote-http,
but git-remote-http is initiating the http connection with the url defined
for remote (remote->url) rather than the passed url. This undermines the
purpose of a config like

	url = https://example.com/repo.git
	pushurl = https://user@example.com/repo.git

Introduced around 888692b7 - CC'ing Tay Ray Chuan. (I don't know if it was
working before, though.)

^ permalink raw reply


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