Git development
 help / color / mirror / Atom feed
* Re: [PATCH] commit: allow {--amend|-c foo} when {HEAD|foo} has empty message
From: Junio C Hamano @ 2012-02-28 21:12 UTC (permalink / raw)
  To: Jeff King; +Cc: Thomas Rast, Thomas Rast, git
In-Reply-To: <20120228195931.GE11260@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Yeah, I agree that treating it like an empty body is reasonable
> (possibly with a warning). But given that nobody has actually seen this
> in the wild, maybe it is simpler to mark it with fsck, and to just die()
> when we see it. That would hopefully alert the author of the broken tool
> early, before the tools is made public. If it turns out that such
> commits do end up in the wild, then we can relax the behavior then.

Yeah, it is not like we would want to encourage a commit with empty body,
be it preceded with "\n\n" or just a "\n", in the first place.

We would need to locate all the places that expect that strstr("\n\n")
will find something, and add die("commit made with a broken git") at these
places anyway, in addition to the change to fsck. Given that, I suspect
that the extra amount of the work needed to tweak the code to tolerate
such a commit and keep going might not be so big, and going that route
would avoid punishing the users of broken versions of git (or broken
imitations of git, for that matter), so...

^ permalink raw reply

* Re: git (commit|tag) atomicity
From: Jon Jagger @ 2012-02-28 20:43 UTC (permalink / raw)
  To: git
In-Reply-To: <4F4D2391.9020701@zabbo.net>

On Tue, Feb 28, 2012 at 6:57 PM, Zach Brown <zab@zabbo.net> wrote:
>
> It's a bit of a tangent, but just to be sure people don't get the wrong
> impression..
>
>
>> But I am not sure... that probably depends on how opendir(3) and
>> readdir(3) works on given filesystem wrt. updates to opened directory.
>> I think VFS on Linux ensures that you see view of filesystem as it was
>> on opendir().
>
>
> No, readdir() does not give you a static view of the entries in a
> directory as it was on opendir().  readdir() will reflect modifications
> that are done after opendir().  The specifics for a given situation
> depend on how the file system maps the readdir position (f_pos) to
> directory entries.  You can see very different results when comparing,
> say, stock ext2, indexed ext[34], and btrfs.

Ok. Thanks.

^ permalink raw reply

* Re: Stash during incomplete merge
From: Neal Kreitzinger @ 2012-02-28 20:22 UTC (permalink / raw)
  To: Phil Hord; +Cc: git@vger.kernel.org, Phil Hord
In-Reply-To: <4F4A7BC7.5010702@cisco.com>

On 2/26/2012 12:36 PM, Phil Hord wrote:
> Hi list,
>
> I was cherry-picking changes from an old branch recently when I ran into
> unexpected behavior with git stash pop.  When I git-stash-save after
> resolving a  merge-conflict, the subsequent git-stash-pop does not
> restore my index.
>
> I think it is the same problem being asked about here:
> http://stackoverflow.com/questions/9009354/git-stash-during-a-merge-conflict
>
> Is this expected behavior or a bug?
>
> <http://stackoverflow.com/questions/9009354/git-stash-during-a-merge-conflict>Here's
> a script the demonstrates the anomaly, but my actual encounter involved
> more files, some of which I added to the index and some I did not:
>
> # Create a sample merge-conflict
> git init  tmp-repo&&  cd tmp-repo
> echo foo>  foo.txt&&  git add foo.txt&&  git commit -m "foo"
> git checkout -b A master&&  echo foo-A>  foo.txt&&  git commit -am "foo-A"
> git checkout -b B master&&  echo foo-B>  foo.txt&&  git commit -am "foo-B"
> git merge A
> git status
> # Resolve the conflict
> echo foo-AB>  foo.txt&&  git add foo.txt
> git status
> git stash
> # test test test...  Resume...
> git stash pop
>
>
> Here's some of the final output:
>
> $ git merge A
> Auto-merging foo.txt
> CONFLICT (content): Merge conflict in foo.txt
> Recorded preimage for 'foo.txt'
> Automatic merge failed; fix conflicts and then commit the result.
>
> $ git status
> # On branch B
> # Unmerged paths:
> #   (use "git add/rm<file>..." as appropriate to mark resolution)
> #
> #       both modified:      foo.txt
> #
> no changes added to commit (use "git add" and/or "git commit -a")
>
> $ # Resolve the conflict
> $ echo foo-AB>  foo.txt&&  git add foo.txt
> $ git status
> # On branch B
> # Changes to be committed:
> #
> #       modified:   foo.txt
> #
>
> $ # Now foo.txt is in my index.  But I have to test something before I
> commit.
> $ git stash
> Saved working directory and index state WIP on B: 80f2a13 foo-B
> HEAD is now at 80f2a13 foo-B
>
> $ # test test test...  Resume...
> $ git stash pop
>
> # On branch B
> # Changes not staged for commit:
> #   (use "git add<file>..." to update what will be committed)
> #   (use "git checkout --<file>..." to discard changes in working
> directory)
> #
> #       modified:   foo.txt
> #
> no changes added to commit (use "git add" and/or "git commit -a")
> Dropped refs/stash@{0} (460a6d5c67a3db613fd27f1854ecc7b89eeaa207)
>
Was the foo.txt in your worktree still the same as the one you resolved 
and added to the index?

If so, at that point you can just do "git add foo.txt" to put it back in 
your index.

(merge conflict)
git add conflict-resolution (file is in worktree and index)
git stash (file is in stash of worktree and stash of index)
git stash pop (file is back in worktree but not back in index)
git add conflict-resolution (file is back in the index)

If not, then you must have a more complicated case like a modified 
worktree version that differs from the index version (original conflict 
resolution) in which case you need to also consider whether or not it is 
an [a]"evil merge".  Ideally, you intend to commit the index (conflict 
resolution) as the merge commit and add any worktree mods in addition to 
that as a commit after the merge commit. If so, then I see your problem. 
The next time, you can get both the worktree version and the index 
version back with:

git stash apply --index

git stash does not apply the index by default so you have to specify the 
--index option if you want the index back also.  I recommend git stash 
apply instead of git stash pop in case you realize you forgot the index. 
Then you can rerun git stash apply --index.  git stash pop can only be 
run once because it throws away the stash after popping it.  You can run 
git stash drop after you confirm that you did the git stash apply 
correctly.

I assume you know that the stash is a stack and how to specify which 
stash you want by using the reflog syntax (see git-stash manpage: 
http://schacon.github.com/git/git-stash.html).  Stashes are really 
[b]"commit objects" of the worktree and index trees (you can see them in 
gitk by viewing all refs), but they are treated according to "stash 
rules" instead of "commit rules".

If you still want to try and get that conflict-resolution from the 
"lost" index from that stash you popped you can try this variation of 
the "Recovering stashes that were cleared/dropped erroneously" procedure 
at the end of manpage:

(1) Find the lost stash:
$ git fsck --unreachable | grep commit | cut -d" "  -f3 | xargs git log 
--no-walk --grep=WIP --grep="index on"

(2) Review the list of commits and decide which pair is for the stash 
you want.  Note on stash commit-object messages:
WIP = stash commit-object of the worktree
"index on" = stash commit-object of the index (this may not exist of the 
index didn't differ from the worktree)

(3) Reset the conflict-resolution-file(s) from the stash of the index:

$ git reset <sha1-of-stash-commit-object-of-index> -- 
<conflict-resolution-file>

git reset will just put it back in your index and leave the worktree alone.

You may also want to consider the --keep-index option on your "git stash 
save" if your "testing" workflow doesn't involve adds or commits before 
the git stash apply/pop.

Footnotes:
a. If you resolved the conflict and then made further mods that is 
called an "evil merge" because you really didn't just merge the changes 
of the two parents, but also made additional changes that were not part 
of the changes made by either of the parents your are merging.  The 
additional changes really should be their own commit after the merge. 
"Evil merges" are misleading (and also probably not documented in the 
merge commit message with a git commit --amend).
b. There are only 4 types of objects in git: tag, commit, tree, blob.

Hope this helps. Maybe someone else has a better way to do this.  Maybe 
my assumptions are incorrect and I'm missing something about what you 
are trying to do.

v/r,
neal

^ permalink raw reply

* Re: [PATCH] Documentation: use {asterisk} in rev-list-options.txt when needed
From: Carlos Martín Nieto @ 2012-02-28 20:20 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20120228194551.GC11725@sigill.intra.peff.net>

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

On Tue, 2012-02-28 at 14:45 -0500, Jeff King wrote:
> On Tue, Feb 28, 2012 at 04:35:48PM +0100, Carlos Martín Nieto wrote:
> 
> > Text between to '*' is emphasized in AsciiDoc which made the
> 
> s/to/two/

Oops. Thanks. Can you squash that in, Junio?

> 
> > glob-related explanations in rev-list-options.txt very confusing, as
> > the rendered text would be missing two asterisks and the text between
> > them would be emphasized instead.
> > 
> > Use '{asterisk}' where needed to make them show up as asterisks in the
> > rendered text.
> > [...]
> > -	'*', or '[', '/*' at the end is implied.
> > +	'{asterisk}', or '[', '/{asterisk}' at the end is implied.
> 
> Ugh. I hate asciidoc more with each passing year. Readable source
> documents are such a wonderful idea, but the markup makes it less and
> less readable as we accumulate fixes like this.  I wonder if this has
> always been a bug, or something that appeared in more recent versions of
> the toolchain.

The generated documentation in the 'html' branch shows the wrong
formatting as well even for 1.7.0 when the --glob feature and its
explanation was first introduced. So either nobody reads the
documentation or very few people actually care about --glob and use the
--remotes and friends, where that part of the explanation isn't that
interesting.

   cmn



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* [PATCH 4/3] parse-options: disallow --no-no-sth
From: René Scharfe @ 2012-02-28 20:12 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Thomas Rast, git, Bert Wesarg, Geoffrey Irving,
	Johannes Schindelin, Pierre Habouzit, Jeff King
In-Reply-To: <7vzkc457g3.fsf@alter.siamese.dyndns.org>

Now that options whose definition starts with "no-" can be negated
by removing said "no-", there is no need anymore to allow them to
be negated by adding a second "no-", which just looks silly.

The following thirteen options are affected:

	apply          --no-add
	bisect--helper --no-checkout
	checkout-index --no-create
	clone          --no-checkout --no-hardlinks
	commit         --no-verify   --no-post-rewrite
	format-patch   --no-binary
	hash-object    --no-filters
	read-tree      --no-sparse-checkout
	revert         --no-commit
	show-branch    --no-name
	update-ref     --no-deref

E.g., with this patch --no-add and --add (its reverse) are still
accepted by git apply, but --no-no-add isn't anymore.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 parse-options.c          |    3 +++
 t/t0040-parse-options.sh |    4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 1908996..dc59bba 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -248,6 +248,9 @@ is_abbreviated:
 				}
 				continue;
 			}
+			/* double negation? */
+			if (!prefixcmp(long_name, "no-"))
+				continue;
 			flags |= OPT_UNSET;
 			rest = skip_prefix(arg + 3, long_name);
 			/* abbreviated and negated? */
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index a44bcb9..b124f3c 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -101,11 +101,11 @@ test_expect_success 'OPT_BOOL() #5' 'check boolean: 1 -B'
 test_expect_success 'OPT_BOOL() is idempotent #1' 'check boolean: 1 --yes --yes'
 test_expect_success 'OPT_BOOL() is idempotent #2' 'check boolean: 1 -DB'
 
-test_expect_success 'OPT_BOOL() negation #1' 'check boolean: 0 -D --no-yes'
-test_expect_success 'OPT_BOOL() negation #2' 'check boolean: 0 -D --no-no-doubt'
+test_expect_success 'OPT_BOOL() negation' 'check boolean: 0 -D --no-yes'
 
 test_expect_success 'OPT_BOOL() no negation #1' 'check_unknown --fear'
 test_expect_success 'OPT_BOOL() no negation #2' 'check_unknown --no-no-fear'
+test_expect_success 'OPT_BOOL() no negation #3' 'check_unknown --no-no-doubt'
 
 test_expect_success 'OPT_BOOL() positivation' 'check boolean: 0 -D --doubt'
 
-- 
1.7.9.2

^ permalink raw reply related

* Re: [PATCH] commit: allow {--amend|-c foo} when {HEAD|foo} has empty message
From: Jeff King @ 2012-02-28 19:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, Thomas Rast, git
In-Reply-To: <7vr4xe27sq.fsf@alter.siamese.dyndns.org>

On Tue, Feb 28, 2012 at 09:21:09AM -0800, Junio C Hamano wrote:

> Thomas Rast <trast@inf.ethz.ch> writes:
> 
> > So either there's a lot to be fixed, or fsck needs to catch this.
> 
> Your experiment with hash-object aside (that is like saying "I can write
> garbage with a disk editor, and now OS cannot read from that directory"),

Yes, but the difference between "OS cannot read from that directory" and
"OS segfaults" might be worth noticing. :)

> if somebody manages to create a commit without any body, it is clear that
> the user wanted to record no body.  I think all code that tries to run
> strstr("\n\n") and increment the resulting pointer by two to find the
> beginning of the body should behave as if it found one and the result
> pointed at a NUL.  Rejecting with fsck does not help anybody, as it
> happens after the fact.

Yeah, I agree that treating it like an empty body is reasonable
(possibly with a warning). But given that nobody has actually seen this
in the wild, maybe it is simpler to mark it with fsck, and to just die()
when we see it. That would hopefully alert the author of the broken tool
early, before the tools is made public. If it turns out that such
commits do end up in the wild, then we can relax the behavior then.

-Peff

^ permalink raw reply

* Re: Delivery Status Notification (Failure)
From: Jeff King @ 2012-02-28 19:48 UTC (permalink / raw)
  To: Rajat Khanduja; +Cc: git
In-Reply-To: <CAAymXMrrQakreKgSjrqBGM4ea1fH8cRWPE=7PbcxPi4dz_hxtw@mail.gmail.com>

On Wed, Feb 29, 2012 at 12:32:37AM +0530, Rajat Khanduja wrote:

> I am a student interested in participating in GSOC'12 and was curious
> to know if Git is participating in the program this year.

We haven't applied yet, but I think we are planning on it. Getting our
application started is on my to-do list this week.

-Peff

^ permalink raw reply

* Re: [PATCH] Documentation: use {asterisk} in rev-list-options.txt when needed
From: Jeff King @ 2012-02-28 19:45 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: git
In-Reply-To: <1330443348-5742-1-git-send-email-cmn@elego.de>

On Tue, Feb 28, 2012 at 04:35:48PM +0100, Carlos Martín Nieto wrote:

> Text between to '*' is emphasized in AsciiDoc which made the

s/to/two/

> glob-related explanations in rev-list-options.txt very confusing, as
> the rendered text would be missing two asterisks and the text between
> them would be emphasized instead.
> 
> Use '{asterisk}' where needed to make them show up as asterisks in the
> rendered text.
> [...]
> -	'*', or '[', '/*' at the end is implied.
> +	'{asterisk}', or '[', '/{asterisk}' at the end is implied.

Ugh. I hate asciidoc more with each passing year. Readable source
documents are such a wonderful idea, but the markup makes it less and
less readable as we accumulate fixes like this.  I wonder if this has
always been a bug, or something that appeared in more recent versions of
the toolchain.

Anyway, that is not a problem with your patch. :) I confirmed that the
bug happens in my version of the toolchain, and your fix works (I also
tried using `*`, but backtick does not suppress markup. It would be nice
if there was an easy marker for "this is a literal name: no markup, tt
font, etc", but I don't think that exists).

Acked-by: Jeff King <peff@peff.net>

-Peff

^ permalink raw reply

* Re: [PATCH (BUGFIX)] gitweb: Handle invalid regexp in regexp search
From: Junio C Hamano @ 2012-02-28 19:45 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Ramsay Jones
In-Reply-To: <20120228183919.26435.86795.stgit@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> writes:

> When using regexp search ('sr' parameter / $search_use_regexp variable
> is true), check first that regexp is valid.

Thanks.

How old is this bug?  Should it go to older maitenance tracks like 1.7.6?

^ permalink raw reply

* Re: [PATCH] Documentation: use {asterisk} in rev-list-options.txt when needed
From: Junio C Hamano @ 2012-02-28 19:38 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: git
In-Reply-To: <1330443348-5742-1-git-send-email-cmn@elego.de>

Thanks.

^ permalink raw reply

* Re: [PATCH 2/3] http: try standard proxy env vars when http.proxy config option is not set
From: Jeff King @ 2012-02-28 19:34 UTC (permalink / raw)
  To: Sam Vilain; +Cc: Nelson Benitez Leon, Thomas Rast, git, sam.vilain
In-Reply-To: <4F4D2AAD.3040107@vilain.net>

On Tue, Feb 28, 2012 at 11:27:41AM -0800, Sam Vilain wrote:

> On 2/28/12 11:15 AM, Jeff King wrote:
> >Usually we would prefer environment variables to config. So that:
> >
> >   $ git config http.proxy foo
> >   $ HTTP_PROXY=bar git fetch
> >
> >would use "bar" as the proxy, not "foo". But your code above would
> >prefer "foo", right?
> 
> Apparently I'm the author of the http.proxy feature, though I barely
> remember what problem I was actually solving at the time.  At the
> time I justified it on the grounds that a user might want to use a
> different proxy for git and/or a particular remote.  The "http_proxy"
> environment variable is likely to be a global system default, or
> perhaps a desktop setting, and therefore I'd say probably less and
> not more specific than a git configuration variable.

Good point. We sometimes follow this order:

  1. git-specific environment variables (i.e., $GIT_HTTP_PROXY, if
     it existed)
  2. git config files (i.e., http.proxy)
  3. generic system environment (i.e., $http_proxy).

So thinking about it that way, the original patch makes more sense.

-Peff

^ permalink raw reply

* Re: [msysGit] Re: [PATCH 0/2] submodules: Use relative paths to gitdir and work tree
From: Jens Lehmann @ 2012-02-28 19:33 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Sixt, Git Mailing List, Antony Male, Phil Hord, msysGit,
	Johannes Schindelin
In-Reply-To: <7vehtezs6q.fsf@alter.siamese.dyndns.org>

Am 28.02.2012 20:14, schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> With the following patch on top of your always-use-relative-gitdir branch
>> from https://github.com/jlehmann/git-submod-enhancements the tests pass
>> on Windows.
>>
>> Thanks, Dscho, for pointing out the obvious.
> 
> The patch looks unintrusive and sane.
> 
> Thanks all three of you for looking into this.  Should I wait for a patch
> with nice write-up from one of you, or should I just come up with a random
> message and apply it locally avoiding roundtrip cost?

Thanks, but that interdiff needs all three patches from my branch to work
properly, while I only posted the first two here so far (without the third
one the gitfile still might contain the "c:/" notation even with J6t's diff
applied). I still need to remove the iffiness of my 2/2 patch and the third
one needs a test case too before I can repost that series.

>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index e1984e0..953ca5e 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -151,6 +151,9 @@ module_clone()
>>  
>>  	a=$(cd "$gitdir" && pwd)
>>  	b=$(cd "$path" && pwd)
>> +	# normalize Windows-style absolute paths to POSIX-style absolute paths
>> +	case $a in [a-zA-Z]:/*) a=/${a%%:*}${a#*:} esac
>> +	case $b in [a-zA-Z]:/*) b=/${b%%:*}${b#*:} esac
>>  	# Remove all common leading directories
>>  	while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
>>  	do
> 

^ permalink raw reply

* Re: [PATCH 3/3] http: when proxy url has username but no password, ask for password
From: Jeff King @ 2012-02-28 19:31 UTC (permalink / raw)
  To: Nelson Benitez Leon; +Cc: git, sam
In-Reply-To: <4F4CCEFD.90402@seap.minhap.es>

On Tue, Feb 28, 2012 at 01:56:29PM +0100, Nelson Benitez Leon wrote:

> diff --git a/http.c b/http.c
> index 79cbe50..68e3f7d 100644
> --- a/http.c
> +++ b/http.c
> @@ -306,7 +306,41 @@ static CURL *get_curl_handle(void)
>  		}
>  	}
>  	if (curl_http_proxy) {
> -		curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
> +		char *at, *colon, *proxyuser;
> +		const char *cp;
> +		cp = strstr(curl_http_proxy, "://");
> +		if (cp == NULL) {
> +			cp = curl_http_proxy;
> +		} else {
> +			cp += 3;
> +		}
> +		at = strchr(cp, '@');
> +		colon = strchr(cp, ':');
> +		if (at && (!colon || at < colon)) {
> +			/* proxy string has username but no password, ask for password */

Don't parse the URL by hand. Use credential_from_url, which will do it
for you (and will properly handle things like unquoting the various
components).

> +			char *ask_str, *proxyuser, *proxypass;

Shouldn't these be static globals? If we have multiple curl handles, you
would want them to share the authentication information we collect here,
and not have to ask the user again, no?

> +			strbuf_addf(&pbuf, "Enter password for proxy %s...", at+1);
> +			ask_str = strbuf_detach(&pbuf, NULL);
> +			proxypass = xstrdup(git_getpass(ask_str));

And this should be using credential_fill(), which will let it use
credential helpers to save passwords, give it the same type of prompt as
elsewhere, etc.

See Documentation/technical/api-credential.txt, and see how regular http
auth is handled for an example.

-Peff

^ permalink raw reply

* Re: [PATCH 2/3] http: try standard proxy env vars when http.proxy config option is not set
From: Sam Vilain @ 2012-02-28 19:27 UTC (permalink / raw)
  To: Jeff King; +Cc: Nelson Benitez Leon, Thomas Rast, git, sam.vilain
In-Reply-To: <20120228191514.GD11260@sigill.intra.peff.net>

On 2/28/12 11:15 AM, Jeff King wrote:
> Usually we would prefer environment variables to config. So that:
>
>    $ git config http.proxy foo
>    $ HTTP_PROXY=bar git fetch
>
> would use "bar" as the proxy, not "foo". But your code above would
> prefer "foo", right?

Apparently I'm the author of the http.proxy feature, though I barely 
remember what problem I was actually solving at the time.  At the time I 
justified it on the grounds that a user might want to use a different 
proxy for git and/or a particular remote.  The "http_proxy" environment 
variable is likely to be a global system default, or perhaps a desktop 
setting, and therefore I'd say probably less and not more specific than 
a git configuration variable.

As to this matter of "HTTP_PROXY", I'm not sure about whether that helps 
or confuses matters to support.  I must admit I'm still confused by the 
motivation of this patch series.

Sam

^ permalink raw reply

* Re: [PATCH 2/3] http: try standard proxy env vars when http.proxy config option is not set
From: Junio C Hamano @ 2012-02-28 19:24 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Nelson Benitez Leon, git, peff, sam.vilain, sam
In-Reply-To: <878vjn8823.fsf@thomas.inf.ethz.ch>

Thomas Rast <trast@inf.ethz.ch> writes:

> Which raises the questions:
>
> * Why is this needed?  Does git's use of libcurl ignore http_proxy?  [1]
>   seems to indicate that libcurl respects <protocol>_proxy
>   automatically.
>
> * Why do you (need to?) support HTTP_PROXY when curl doesn't?

Let me add a third bullet point.

I've heard rumors that libcurl on some versions/installations of Mac OS X
deliberately ignores the environment. For those who agree with Apple, it
would be a regression if we suddenly start the environment ourselves and
using it.

^ permalink raw reply

* Re: [msysGit] Re: [PATCH 0/2] submodules: Use relative paths to gitdir and work tree
From: Jens Lehmann @ 2012-02-28 19:21 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Junio C Hamano, Git Mailing List, Antony Male, Phil Hord, msysGit,
	Johannes Schindelin
In-Reply-To: <4F4D23D8.1050208@kdbg.org>

Am 28.02.2012 19:58, schrieb Johannes Sixt:
> Am 27.02.2012 22:19, schrieb Johannes Sixt:
>> Am 26.02.2012 20:58, schrieb Jens Lehmann:
>>> -       gitdir=$(git rev-parse --git-dir)
>>> +       gitdir=$(git rev-parse --git-dir | sed -e 's,^\([a-z]\):/,/\1/,')
>>
>> I don't like pipelines of this kind because they fork yet another
>> process. But it looks like there are not that many alternatives...
> 
> With the following patch on top of your always-use-relative-gitdir branch
> from https://github.com/jlehmann/git-submod-enhancements the tests pass
> on Windows.
> 
> Thanks, Dscho, for pointing out the obvious.

Thanks for helping to test and fix that on the Windows side. Do you want
to post a commit based on the the interdiff below so I can apply it on
top of my branch? Then I would make this a four patch series in the next
round.

> diff --git a/git-submodule.sh b/git-submodule.sh
> index e1984e0..953ca5e 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -151,6 +151,9 @@ module_clone()
>  
>  	a=$(cd "$gitdir" && pwd)
>  	b=$(cd "$path" && pwd)
> +	# normalize Windows-style absolute paths to POSIX-style absolute paths
> +	case $a in [a-zA-Z]:/*) a=/${a%%:*}${a#*:} esac
> +	case $b in [a-zA-Z]:/*) b=/${b%%:*}${b#*:} esac
>  	# Remove all common leading directories
>  	while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
>  	do
> 

^ permalink raw reply

* Re: Tilde spec - befuzzled
From: Junio C Hamano @ 2012-02-28 19:20 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Andreas Ericsson, Luke Diamand, Git List
In-Reply-To: <87zkc38a3v.fsf@thomas.inf.ethz.ch>

Thomas Rast <trast@inf.ethz.ch> writes:

>>> '<rev>{tilde}<n>', e.g. 'master{tilde}3'::
>>> A suffix '{tilde}<n>' to a revision parameter means the commit
>>> object that is the <n>th generation grand-parent of the named
>>> commit object, following only the first parents.
>>> 
>>> Hang on, *grand*-parents?
>>> ...
>
> Perhaps we should reword it as "n-th first-parent ancestor"?  Barring
> confusion about the position of the dashes, that leaves little room for
> error.

I think we could either go "easier to read but not precise"

	... that is the <n>th generation (grand-)parent of ...

or "may sound scary but correct"

	the ancestor reached by walking the first-parent chain <n> times

I am not sure which bucket "n-th first-parent ancestor" falls into.

^ permalink raw reply

* Re: [PATCH 2/3] http: try standard proxy env vars when http.proxy config option is not set
From: Jeff King @ 2012-02-28 19:15 UTC (permalink / raw)
  To: Nelson Benitez Leon; +Cc: Thomas Rast, git, sam.vilain, sam
In-Reply-To: <4F4CCE8A.4010800@seap.minhap.es>

On Tue, Feb 28, 2012 at 01:54:34PM +0100, Nelson Benitez Leon wrote:

> diff --git a/http.c b/http.c
> index 8ac8eb6..79cbe50 100644
> --- a/http.c
> +++ b/http.c
> @@ -295,6 +295,16 @@ static CURL *get_curl_handle(void)
>  	if (curl_ftp_no_epsv)
>  		curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0);
>  
> +	if (!curl_http_proxy) {
> +		const char *env_proxy;
> +		env_proxy = getenv("HTTP_PROXY");
> +		if (!env_proxy) {
> +			env_proxy = getenv("http_proxy");
> +		}
> +		if (env_proxy) {
> +			curl_http_proxy = xstrdup(env_proxy);
> +		}
> +	}

Usually we would prefer environment variables to config. So that:

  $ git config http.proxy foo
  $ HTTP_PROXY=bar git fetch

would use "bar" as the proxy, not "foo". But your code above would
prefer "foo", right?

>From reading Thomas's messages, I think there is a slight complication
in that right now curl is respecting $http_proxy, and it is probably
letting git's http.proxy overwrite (though I didn't check). If that is
the case, then that is IMHO a bug that should be fixed. So the rationale
for this patch would be three-fold:

  1. Support HTTP_PROXY, which curl does not accept.

  2. Fix the precedence of environment variables over config.

  3. By handling the proxy variables ourselves, we have more flexibility
     in handling the authentication.

-Peff

^ permalink raw reply

* Re: [msysGit] Re: [PATCH 0/2] submodules: Use relative paths to gitdir and work tree
From: Junio C Hamano @ 2012-02-28 19:14 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Jens Lehmann, Junio C Hamano, Git Mailing List, Antony Male,
	Phil Hord, msysGit, Johannes Schindelin
In-Reply-To: <4F4D23D8.1050208@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

> With the following patch on top of your always-use-relative-gitdir branch
> from https://github.com/jlehmann/git-submod-enhancements the tests pass
> on Windows.
>
> Thanks, Dscho, for pointing out the obvious.

The patch looks unintrusive and sane.

Thanks all three of you for looking into this.  Should I wait for a patch
with nice write-up from one of you, or should I just come up with a random
message and apply it locally avoiding roundtrip cost?

> diff --git a/git-submodule.sh b/git-submodule.sh
> index e1984e0..953ca5e 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -151,6 +151,9 @@ module_clone()
>  
>  	a=$(cd "$gitdir" && pwd)
>  	b=$(cd "$path" && pwd)
> +	# normalize Windows-style absolute paths to POSIX-style absolute paths
> +	case $a in [a-zA-Z]:/*) a=/${a%%:*}${a#*:} esac
> +	case $b in [a-zA-Z]:/*) b=/${b%%:*}${b#*:} esac
>  	# Remove all common leading directories
>  	while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
>  	do

^ permalink raw reply

* Re: [PATCH v7 02/10] Stop starting pager recursively
From: Junio C Hamano @ 2012-02-28 19:10 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Ramsay Jones
In-Reply-To: <7v4nua25cz.fsf@alter.siamese.dyndns.org>

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

> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> git-column can be used as a pager for other git commands, something
>> like this:
>>
>>     GIT_PAGER="git -p column --mode='dense color'" git -p branch
>>
>> The problem with this is that "git -p column" also has $GIT_PAGER
>> set so the pager runs itself again as a pager, then again and again.
>>
>> Stop this.
>
> A natural question that may come after reading only the above is if "git
> column" is the only one that has this problem.  In other words, is the
> undesirable behaviour you observed caused by a bug in setup_pager() that
> needs to be fixed, or should it be fixed in "git column"?

Put another way, if there is another git command X that can be used as a
filter to the output of a git command Y, do you suffer from the same issue
to when you abuse the GIT_PAGER mechanism to pipe the output from Y to X?
That is a sure sign that the pager mechanism needs improvement (obviously,
an alternative answer could be "don't do that then", though).

For example, shortlog is designed to be X for Y=log, i.e.

	$ git log v1.0.0.. | git shortlog

is a perfectly valid way to use the command.  I could imagine that this
patch may improve the situation if you abuse GIT_PAGER mechanism to
implement the above pipeline, i.e.

	$ GIT_PAGER="git -p shortlog" git log v1.0.0..

Although I never tried it.

^ permalink raw reply

* Re: [PATCH 3/3 v2] parse-options: remove PARSE_OPT_NEGHELP
From: Jeff King @ 2012-02-28 19:09 UTC (permalink / raw)
  To: René Scharfe
  Cc: git, Junio C Hamano, Bert Wesarg, Geoffrey Irving,
	Johannes Schindelin, Pierre Habouzit
In-Reply-To: <4F4D25A1.8050702@lsrfire.ath.cx>

On Tue, Feb 28, 2012 at 08:06:09PM +0100, René Scharfe wrote:

> PARSE_OPT_NEGHELP is confusing because short options defined with that
> flag do the opposite of what the helptext says. It is also not needed
> anymore now that options starting with no- can be negated by removing
> that prefix. Convert its only two users to OPT_NEGBIT() and OPT_BOOL()
> and then remove support for PARSE_OPT_NEGHELP.
> 
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
> ---
> This version doesn't invert the logic in grep anymore.

Thanks, I like this one much better.

-Peff

^ permalink raw reply

* [PATCH 3/3 v2] parse-options: remove PARSE_OPT_NEGHELP
From: René Scharfe @ 2012-02-28 19:06 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Bert Wesarg, Geoffrey Irving, Johannes Schindelin,
	Pierre Habouzit, Jeff King
In-Reply-To: <4F49336C.3000303@lsrfire.ath.cx>

PARSE_OPT_NEGHELP is confusing because short options defined with that
flag do the opposite of what the helptext says. It is also not needed
anymore now that options starting with no- can be negated by removing
that prefix. Convert its only two users to OPT_NEGBIT() and OPT_BOOL()
and then remove support for PARSE_OPT_NEGHELP.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
This version doesn't invert the logic in grep anymore.

 builtin/fast-export.c |    4 +---
 builtin/grep.c        |    5 ++---
 parse-options.c       |    6 ++----
 parse-options.h       |    4 ----
 4 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 08fed98..19509ea 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -647,9 +647,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 			     "Output full tree for each commit"),
 		OPT_BOOLEAN(0, "use-done-feature", &use_done_feature,
 			     "Use the done feature to terminate the stream"),
-		{ OPTION_NEGBIT, 0, "data", &no_data, NULL,
-			"Skip output of blob data",
-			PARSE_OPT_NOARG | PARSE_OPT_NEGHELP, NULL, 1 },
+		OPT_BOOL(0, "no-data", &no_data, "Skip output of blob data"),
 		OPT_END()
 	};
 
diff --git a/builtin/grep.c b/builtin/grep.c
index e4ea900..643938d 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -684,9 +684,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	struct option options[] = {
 		OPT_BOOLEAN(0, "cached", &cached,
 			"search in index instead of in the work tree"),
-		{ OPTION_BOOLEAN, 0, "index", &use_index, NULL,
-			"finds in contents not managed by git",
-			PARSE_OPT_NOARG | PARSE_OPT_NEGHELP },
+		OPT_NEGBIT(0, "no-index", &use_index,
+			 "finds in contents not managed by git", 1),
 		OPT_BOOLEAN(0, "untracked", &untracked,
 			"search in both tracked and untracked files"),
 		OPT_SET_INT(0, "exclude-standard", &opt_exclude,
diff --git a/parse-options.c b/parse-options.c
index 8906841..1908996 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -533,7 +533,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
 			continue;
 
 		pos = fprintf(outfile, "    ");
-		if (opts->short_name && !(opts->flags & PARSE_OPT_NEGHELP)) {
+		if (opts->short_name) {
 			if (opts->flags & PARSE_OPT_NODASH)
 				pos += fprintf(outfile, "%c", opts->short_name);
 			else
@@ -542,9 +542,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
 		if (opts->long_name && opts->short_name)
 			pos += fprintf(outfile, ", ");
 		if (opts->long_name)
-			pos += fprintf(outfile, "--%s%s",
-				(opts->flags & PARSE_OPT_NEGHELP) ?  "no-" : "",
-				opts->long_name);
+			pos += fprintf(outfile, "--%s", opts->long_name);
 		if (opts->type == OPTION_NUMBER)
 			pos += fprintf(outfile, "-NUM");
 
diff --git a/parse-options.h b/parse-options.h
index 2e811dc..def9ced 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -40,7 +40,6 @@ enum parse_opt_option_flags {
 	PARSE_OPT_LASTARG_DEFAULT = 16,
 	PARSE_OPT_NODASH = 32,
 	PARSE_OPT_LITERAL_ARGHELP = 64,
-	PARSE_OPT_NEGHELP = 128,
 	PARSE_OPT_SHELL_EVAL = 256
 };
 
@@ -90,9 +89,6 @@ typedef int parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
  *   PARSE_OPT_LITERAL_ARGHELP: says that argh shouldn't be enclosed in brackets
  *				(i.e. '<argh>') in the help message.
  *				Useful for options with multiple parameters.
- *   PARSE_OPT_NEGHELP: says that the long option should always be shown with
- *				the --no prefix in the usage message. Sometimes
- *				useful for users of OPTION_NEGBIT.
  *
  * `callback`::
  *   pointer to the callback to use for OPTION_CALLBACK or
-- 
1.7.9.2

^ permalink raw reply related

* Re: git (commit|tag) atomicity
From: Zach Brown @ 2012-02-28 18:57 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Jon Jagger, git, Holger Hellmuth
In-Reply-To: <m3aa42vosb.fsf@localhost.localdomain>


It's a bit of a tangent, but just to be sure people don't get the wrong
impression..

> But I am not sure... that probably depends on how opendir(3) and
> readdir(3) works on given filesystem wrt. updates to opened directory.
> I think VFS on Linux ensures that you see view of filesystem as it was
> on opendir().

No, readdir() does not give you a static view of the entries in a
directory as it was on opendir().  readdir() will reflect modifications
that are done after opendir().  The specifics for a given situation
depend on how the file system maps the readdir position (f_pos) to
directory entries.  You can see very different results when comparing,
say, stock ext2, indexed ext[34], and btrfs.

- z
(your message probably caught my eye because telldir()/seekdir() is
*loathed* by file system designers)

^ permalink raw reply

* Re: Delivery Status Notification (Failure)
From: Rajat Khanduja @ 2012-02-28 19:02 UTC (permalink / raw)
  To: git
In-Reply-To: <001636c927b40af7cb04ba0aca6e@google.com>

Hi

I am a student interested in participating in GSOC'12 and was curious
to know if Git is participating in the program this year.

Sincerely,

Rajat

^ permalink raw reply

* Re: [msysGit] Re: [PATCH 0/2] submodules: Use relative paths to gitdir and work tree
From: Johannes Sixt @ 2012-02-28 18:58 UTC (permalink / raw)
  To: Jens Lehmann
  Cc: Junio C Hamano, Git Mailing List, Antony Male, Phil Hord, msysGit,
	Johannes Schindelin
In-Reply-To: <4F4BF357.8020407@kdbg.org>

Am 27.02.2012 22:19, schrieb Johannes Sixt:
> Am 26.02.2012 20:58, schrieb Jens Lehmann:
>> I don't understand why you need this. Does "pwd" sometimes return a
>> path starting with "c:/" and sometimes "/c/" depending on what form
>> you use when you cd into that directory?
> 
> It looks like this is the case. I was surprised as well. I hoped that
> pwd -P would fix it, but it makes no difference. I should have tested
> pwd -L as well, but I forgot.

pwd -L doesn't make a difference, either.

>> -       gitdir=$(git rev-parse --git-dir)
>> +       gitdir=$(git rev-parse --git-dir | sed -e 's,^\([a-z]\):/,/\1/,')
> 
> I don't like pipelines of this kind because they fork yet another
> process. But it looks like there are not that many alternatives...

With the following patch on top of your always-use-relative-gitdir branch
from https://github.com/jlehmann/git-submod-enhancements the tests pass
on Windows.

Thanks, Dscho, for pointing out the obvious.

diff --git a/git-submodule.sh b/git-submodule.sh
index e1984e0..953ca5e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -151,6 +151,9 @@ module_clone()
 
 	a=$(cd "$gitdir" && pwd)
 	b=$(cd "$path" && pwd)
+	# normalize Windows-style absolute paths to POSIX-style absolute paths
+	case $a in [a-zA-Z]:/*) a=/${a%%:*}${a#*:} esac
+	case $b in [a-zA-Z]:/*) b=/${b%%:*}${b#*:} esac
 	# Remove all common leading directories
 	while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
 	do

^ 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