* Re: [PATCH 1/2] t5540: test DAV push with authentication
From: Jeff King @ 2011-12-13 23:16 UTC (permalink / raw)
To: Sebastian Schuberth; +Cc: Junio C Hamano, Stefan Naewe, Eric, git, msysgit
In-Reply-To: <CAHGBnuO1CFGacMQb+Md_uGsLu=V9bGDpcFLd9Lb4C5jSN+uUiw@mail.gmail.com>
On Tue, Dec 13, 2011 at 10:28:07PM +0100, Sebastian Schuberth wrote:
> On Tue, Dec 13, 2011 at 21:17, Jeff King <peff@peff.net> wrote:
>
> > We don't currently test this case at all, and instead just
> > test the DAV mechanism over an unauthenticated push. That
> > isn't very realistic, as most people will want to
> > authenticate pushes.
>
> Thanks for adding this, Peff!
You're welcome. Thank you for forwarding the bug report. I would never
have seen it on the msysgit list, and for some reason it seems that
msysgit people are more likely to use DAV.
Having looked a lot at the http code the past month or two, I knew it
was pretty flaky and I was nervous when we added Stefan's patch (and no,
I don't blame Stefan; his patch was completely reasonable, but just
happened to trigger a problem in a seldom-looked-at corner of the code).
But I hadn't looked at http-push at all until yesterday, and it didn't
even occur to me that there was another whole area of code relying in a
very obscure way on the http.c auth code. I'll take a look at some of
the refactoring I've done in http.c (both for the credentials topic as
well as the bundle topic) and see if we can't integrate http-push.c a
little more smoothly.
-Peff
^ permalink raw reply
* Re: [PATCH] Revert "http: don't always prompt for password"
From: Jeff King @ 2011-12-13 23:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Naewe, Sebastian Schuberth, Eric, git, msysgit
In-Reply-To: <7v62hkuptt.fsf@alter.siamese.dyndns.org>
On Tue, Dec 13, 2011 at 01:25:18PM -0800, Junio C Hamano wrote:
> > @@ -844,7 +846,7 @@ static int http_request(const char *url, void *result, int target, int options)
> > else if (missing_target(&results))
> > ret = HTTP_MISSING_TARGET;
> > else if (results.http_code == 401) {
> > - if (user_name && user_pass) {
> > + if (user_name) {
> > ret = HTTP_NOAUTH;
> > } else {
> > /*
>
> In the credential series, this is where we declare the given credential is
> to be rejected (if we have both username and password), or ask them to be
> filled by calling credential_fill(), so I think the code in the credential
> series does not need this revert. Right?
Sort of. The point of this conditional is "did we actually send a
credential? If yes, then return HTTP_NOAUTH. Otherwise, ask for the
credential and return HTTP_REAUTH"[1].
Prior to Stefan's patch, if user_name was set, then we sent a credential
(because we always filled in the password if user_name was set). After,
we had to check both (actually, I think checking user_pass would have
been sufficient)).
The situation is the same with credentials, but the variable name is
different. Even though credential_fill will fill in both parts, we may
have set http_auth.username from the URL, but not actually called
credential_fill (and therefore not sent any credentials).
So logically, the revert in the credential series would be:
- if (http_auth.username && http_auth.password)
+ if (http_auth.username)
except that I believe the former is a superset of the latter in both
cases (with or without the credential topic). So leaving it as-is would
be OK. In fact, when reverting Stefan's patch, you could just drop this
hunk entirely, which might be worth it to avoid conflicts with in-flight
topics.
[1] A much saner approach would be to always return HTTP_NOAUTH, and
then let the caller decide to re-ask for credentials and re-try.
But we need the magic curl slot object, which the caller doesn't
have. So doing it that way would have taken some refactoring.
> > @@ -853,8 +855,7 @@ static int http_request(const char *url, void *result, int target, int options)
> > * but that is non-portable. Using git_getpass() can at least be stubbed
> > * on other platforms with a different implementation if/when necessary.
> > */
> > - if (!user_name)
> > - user_name = xstrdup(git_getpass_with_description("Username", description));
> > + user_name = xstrdup(git_getpass_with_description("Username", description));
> > init_curl_http_auth(slot->curl);
> > ret = HTTP_REAUTH;
> > }
>
> So is this one.
Yeah, this code just goes away, as credential_fill() does the right
thing.
But again, the "if (!user_name)" version post-986bbc08 handles both the
pre-986bbc08 condition (because it will always be NULL then) and the
post-986bbc08 (because we do need to check then). So I believe you could
drop the hunk entirely.
Here's a re-roll of the revert that touches as little as possible. I
believe it's correct from my analysis above, and it does pass the tests.
I also included the flipping of the "expect_failure" test, which I
forgot to put in my original patch.
-- >8 --
Subject: [PATCH] Revert "http: don't always prompt for password"
This is a partial revert of commit 986bbc0842334f0e07731fa37f2a55d2930a5b8c.
The rationale for that commit relied on the fact that asking
for the password up-front was merely an optimization,
because git will notice an HTTP 401 and prompt for the
password. However, that is only true for smart-http, and for
dumb fetching. Dumb push over DAV does not have this
feature; as a result, authenticated push-over-DAV does not
work at all, as it never prompts the user for a password.
This is a partial revert that just restores the "don't ask
for password bits". There were some other related
adjustments in 986bbc08 to handle the fact that the
user_name field might be set even if we didn't send a
credential on the first try. The new logic introduced by
986bbc08 handles both the old case and the new case, so we
can leave them intact. That will prevent unnecessary
conflicts with other in-flight topics that touch this code.
Signed-off-by: Jeff King <peff@peff.net>
---
http.c | 2 ++
t/t5540-http-push.sh | 2 +-
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/http.c b/http.c
index 008ad72..33c63b5 100644
--- a/http.c
+++ b/http.c
@@ -279,6 +279,8 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
#endif
+ init_curl_http_auth(result);
+
if (ssl_cert != NULL)
curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
if (has_cert_password())
diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index 3300227..1eea647 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh
@@ -160,7 +160,7 @@ test_expect_success 'PUT and MOVE sends object to URLs with SHA-1 hash suffix' '
test_http_push_nonff "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \
"$ROOT_PATH"/test_repo_clone master
-test_expect_failure 'push to password-protected repository (user in URL)' '
+test_expect_success 'push to password-protected repository (user in URL)' '
test_commit pw-user &&
git push "$HTTPD_URL_USER/auth/dumb/test_repo.git" HEAD &&
git rev-parse --verify HEAD >expect &&
--
1.7.8.17.gfd3524
^ permalink raw reply related
* Re: Auto update submodules after merge and reset
From: Andreas T.Auer @ 2011-12-13 23:05 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Phil Hord, git
In-Reply-To: <4EE7C733.4010209@web.de>
On 13.12.2011 22:44 Jens Lehmann wrote:
> Am 13.12.2011 10:45, schrieb Andreas T.Auer:
> > On 12.12.2011 23:39 Jens Lehmann wrote:
> >> Am 11.12.2011 22:15, schrieb Andreas T.Auer:
> >>> For example
> >>>
> >>> [submodule "commonlib"] update = heads develop:unstable
> >>> master:stable maint:bugfixes
> >>
> >> Having that configured with "branch=unstable", "branch=stable"
> >> etc. in .gitmodules on the superprojects branches would be
> >> simpler and achieve the same functionality.
> >
> > Yes, this has been my first thought also, but there is also a good
> > point to keep the .gitmodules stable, or you always have to change
> > the file when branches change their names. So when the maint branch
> > of version 1.3 become an archive branch and the new maint is on
> > 1.4, which was the master before then you have to change the
> > .gitmodules on these branches. I.e. .gitmodules of 1.4 have
> > "stable" and must have "bugfixes" now and .gitmodules of 1.3 has
> > "bugfixes" and must remove the floating completely. I'm not sure
> > that this can always be solved with "easy" merging and therefore it
> > is probably not really simpler, when you have to do this for every
> > new release. Or what do you think?
>
> I never rename branches, so I do not concur ;-)
Well, maybe you don't, but Junio does something like that. There is a
maint-1.7.7 where maint has been before and maint jumped to master.
> And I think the .gitmodules file could benefit from a special merge
> driver being aware of the format and some subtleties (like not just
> adding a "branch" setting but rather creating a merge conflict)
> anyways.
If that would work it would be fine, but you would still have to create
a new commit, when maint jumps to master and you need to update the
.gitmodules to be a maint .gitmodules.
> So I'd prefer to keep it simple and just use the .gitmodules we
> already have which can be different in different branches.
I agree that the .gitmodule format would be simpler, but I'd prefer the
.gitmodule to be a little bit more complex, but more stable.
> >>> So whenever a defined branch is checked out in the
> >>> superproject the mapped branch will be checked out in the
> >>> submodule ("new" commit), but if a (e.g. tagged) commit is
> >>> checked out ("old" commit) then the gitlink in the superproject
> >>> is used to check out the referenced commit in the submodule.
> >>
> >> I think checkout should only use the submodule commit recorded in
> >> the superproject and a subsequent "git submodule update" should
> >> be needed to update the submodule to tip. Otherwise you record
> >> SHA-1 but still won't be able to bisect ...
> >
> > bisect would leave the branch and therefore uses the recorded SHA1
> > for the submodule checkout instead of the tip. "follow-the-tip"
> > should only work if the superproject follows the tip.
>
> If you follow a tip there won't be any new SHA-1s recorded during
> that following so you could not do a bisect and expect the submodule
> to be what the developer had when doing the commits, no?
If you never commit something to the superproject, you wouldn't get
SHA1s recorded, that's right. But when you commit something to the
superproject, why shouldn't the current submodule SHA1 be stored?
Floating is about _ignoring_ the recorded SHA1 in _some_ cases, not
about disabling the recording. So you can bisect to the bad superproject
commit. If you suspect a bad submodule commit causing the problem then
you could still bisect the submodule commits between the recorded SHA1s.
^ permalink raw reply
* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Marc Branchaud @ 2011-12-13 22:42 UTC (permalink / raw)
To: Jens Lehmann
Cc: Phil Hord, Leif Gruenwoldt, Andreas T.Auer, Junio C Hamano, git
In-Reply-To: <4EE7C15A.3040501@web.de>
On 11-12-13 04:19 PM, Jens Lehmann wrote:
> Am 13.12.2011 16:35, schrieb Marc Branchaud:
>> I'd prefer to have floating be explicitly configured on a per-branch (or
>> per-branch-glob) basis. So in addition to what Jens described yesterday [1]
>> to configure an individual submodule's floating branch, I suggest there also
>> be a new section in the .gitmodules file for configuring the super-repo's
>> floating branches, e.g.
>>
>> [super]
>> floaters = refs/heads/master refs/heads/dev*
>>
>> [submodule "Sub1"]
>> path = foo/bar
>> branch = maint
>> url = ...
>>
>> [submodule "Sub2"]
>> path = other/place
>> url = ...
>
> Hmm, but you can have different .gitmodules files in different branches of
> the superproject, no?
Yes. I'm not sure I see that as a problem though.
> Why not just have the "branch = maint" setting for
> "Sub1" in the master and the dev branches .gitmodules file and drop it in
> the other branches?
Because I think that's an error-prone approach.
If the user creates a topic branch off (an ancestor) of master, git doesn't
know if the user wants floating submodules or not. If this is a bugfix
topic, the user would have to edit .gitmodules to turn off floating. But
that modified .gitmodules is too easily committed to the branch, and once it
gets merged back into master suddenly master loses its "floating" feature.
What's more, less-sophisticated users would be wary of editing an "internal"
file like .gitmodules.
Instead I think it's more intuitive for the repository to define which
branches get floating submodules and which don't, and IMO a list of
names/globs is a good way to do that. The repo's users would need to be
aware of what the magic branch names are, but I think that's easily
communicated in the floating-submodule scenarios I've seen posted. Git could
also help by telling the user, when a branch is created or it's name is
displayed, whether or not it's got floating submodules.
M.
^ permalink raw reply
* [PATCH] Add '-P' as a synonym for '--no-pager' in the git command
From: Joe Ratterman @ 2011-12-13 22:35 UTC (permalink / raw)
To: git; +Cc: Joe Ratterman
In-Reply-To: <1323815706-10560-1-git-send-email-jratt0@gmail.com>
Also, add both -P|--no-pager to the existing -p|--paginate in bash
completion.
Signed-off-by: Joe Ratterman <jratt0@gmail.com>
---
Documentation/git.txt | 3 ++-
contrib/completion/git-completion.bash | 2 +-
git.c | 4 ++--
| 8 ++++++++
4 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index e869032..c7f8445 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -10,7 +10,7 @@ SYNOPSIS
--------
[verse]
'git' [--version] [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]
- [-p|--paginate|--no-pager] [--no-replace-objects] [--bare]
+ [-p|--paginate|-P|--no-pager] [--no-replace-objects] [--bare]
[--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]
[-c <name>=<value>]
[--help] <command> [<args>]
@@ -329,6 +329,7 @@ help ...`.
configuration options (see the "Configuration Mechanism" section
below).
+-P::
--no-pager::
Do not pipe git output into a pager.
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index cc1bdf9..8b9ea1b 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2640,7 +2640,7 @@ _git ()
case "$i" in
--git-dir=*) __git_dir="${i#--git-dir=}" ;;
--bare) __git_dir="." ;;
- --version|-p|--paginate) ;;
+ --version|-p|--paginate|-P|--no-pager) ;;
--help) command="help"; break ;;
*) command="$i"; break ;;
esac
diff --git a/git.c b/git.c
index 8e34903..baa1613 100644
--- a/git.c
+++ b/git.c
@@ -7,7 +7,7 @@
const char git_usage_string[] =
"git [--version] [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]\n"
- " [-p|--paginate|--no-pager] [--no-replace-objects] [--bare]\n"
+ " [-p|--paginate|-P|--no-pager] [--no-replace-objects] [--bare]\n"
" [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]\n"
" [-c name=value] [--help]\n"
" <command> [<args>]";
@@ -103,7 +103,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
exit(0);
} else if (!strcmp(cmd, "-p") || !strcmp(cmd, "--paginate")) {
use_pager = 1;
- } else if (!strcmp(cmd, "--no-pager")) {
+ } else if (!strcmp(cmd, "-P") || !strcmp(cmd, "--no-pager")) {
use_pager = 0;
if (envchanged)
*envchanged = 1;
--git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 320e1d1..783915e 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -84,6 +84,14 @@ test_expect_success 'no pager even with --paginate when stdout is a pipe' '
! test -e paginated.out
'
+test_expect_success TTY 'no pager with -P' '
+ rm -f paginated.out ||
+ cleanup_fail &&
+
+ test_terminal git -P log &&
+ ! test -e paginated.out
+'
+
test_expect_success TTY 'no pager with --no-pager' '
rm -f paginated.out ||
cleanup_fail &&
--
1.7.7.1
^ permalink raw reply related
* [PATCH] Gitweb: Avoid warnings when a repo does not have a valid HEAD
From: Joe Ratterman @ 2011-12-13 22:35 UTC (permalink / raw)
To: git; +Cc: Joe Ratterman
It is possible that the HEAD reference does not point to an existing
branch. When viewing such a repository in gitweb, a message like this
one was sent to the error log:
gitweb.cgi: Use of uninitialized value in string eq at /usr/src/git/gitweb/gitweb.cgi line 5115.
Signed-off-by: Joe Ratterman <jratt0@gmail.com>
---
gitweb/gitweb.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4f0c3bd..5af06d6 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5440,7 +5440,7 @@ sub git_heads_body {
for (my $i = $from; $i <= $to; $i++) {
my $entry = $headlist->[$i];
my %ref = %$entry;
- my $curr = $ref{'id'} eq $head;
+ my $curr = $head ? ($ref{'id'} eq $head) : 0;
if ($alternate) {
print "<tr class=\"dark\">\n";
} else {
--
1.7.7.1
^ permalink raw reply related
* Re: [PATCH v2 28/51] refs.c: rename ref_array -> ref_dir
From: Michael Haggerty @ 2011-12-13 22:13 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips
In-Reply-To: <4EE7A387.3070400@alum.mit.edu>
On 12/13/2011 08:12 PM, Michael Haggerty wrote:
> On 12/13/2011 07:37 AM, Junio C Hamano wrote:
>> Another possibility is to keep the extra_ref interface in refs.c, without
>> any of your changes (i.e. keep it just as a flat list, with the original
>> interface to append to it without any dedup and other fancy features) and
>> also keep the special casing of it in for_each_ref(). AFAIK, that is the
>> only iterator that should care about the extra refs. Thinking about it a
>> bit more, removal of add_extra_ref() API may be unnecessarily complex with
>> no real gain. For example, extra refs added in builtin/clone.c is used by
>> builtin/fetch-pack.c, meaning that the codepath that discovers and
>> remembers these extra history anchor points and the codepath that uses
>> them while walking the history are not localized and we would need some
>> sort of "extra ref API" anyway. I am leaning towards this option.
>
> In a few minutes I will post a series of patches that store extra_refs
> in a linked list separate from the reference caches, and iterates over
> them only from for_each_ref(). I could rebase my ref-api-D changes on
> top of this patch series in such a way that the extra refs are kept in
> non-hierarchical storage. But I leave for vacation on Thursday so it is
> quite likely that I won't be able to get it finished before I return in
> the new year.
I forgot to mention that I think it would be better to separate the
handling of extra refs even more than do the patches
[PATCH 0/6] Handle extra_refs separately from ref_caches
Namely, we should figure out what code wants to set extra refs and or
wants to include extra refs in its iteration over references. The
setters don't have to be changed, but the readers should be. After the
above patch series, the readers call for_each_ref(), which always calls
do_for_each_extra_ref(). The call to do_for_each_extra_ref() should be
taken out of for_each_ref() and called directly by any readers who want
to support extra_refs. Other readers should only call for_each_ref().
(do_for_each_extra_ref(), probably under a different name, would have to
become public.) That way, for_each_ref() could ignore the extra refs
entirely, and other callers of for_each_ref() wouldn't risk getting
extra refs by accident.
Today I spent some time trying to figure out what callers of
for_each_ref() could possible want to deal with extra refs, but I got
lost in the deep call trees and undocumented function declarations. At
the top it is pretty clear that all such code paths start in cmd_clone()
or cmd_receive_pack(). But there are so many calls to the
for_each_ref*() family of functions that I wasn't able to determine
exactly which should allow for extra refs and which shouldn't. (I even
wonder whether extra refs might be used inadvertently in some places
where they aren't wanted, for example in one of the myriad calls to
setup_revisions().) If somebody could put his finger on the
for_each_ref() callers who want to support extra refs, that would be
very helpful.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH] Revert "http: don't always prompt for password"
From: Eric Advincula @ 2011-12-13 21:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Stefan Naewe, Sebastian Schuberth, git, msysgit
In-Reply-To: <7vaa6wuqjt.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 652 bytes --]
Is there an alternative to using git on windows? I used windows, apache,
dav for git.
If there is a better solution please let me know.
Thanks
Eric
On Tue, Dec 13, 2011 at 2:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
> > Doing (3) is obviously the easiest thing. And given the complexity of
> > the other two solutions, I think it makes sense to revert 986bbc08
> > (i.e., apply this patch), ship a working v1.7.8.1, and then look at
> > doing one of the other two solutions for v1.7.9.
>
> Or just let the "dumb HTTP" die.
>
> I thought push over DAV has long been dead; is anybody using it for real?
>
[-- Attachment #2: Type: text/html, Size: 1042 bytes --]
^ permalink raw reply
* Re: Auto update submodules after merge and reset
From: Jens Lehmann @ 2011-12-13 21:44 UTC (permalink / raw)
To: Andreas T.Auer; +Cc: Phil Hord, git
In-Reply-To: <4EE71E9F.90204@ursus.ath.cx>
Am 13.12.2011 10:45, schrieb Andreas T.Auer:
> On 12.12.2011 23:39 Jens Lehmann wrote:
>> Am 11.12.2011 22:15, schrieb Andreas T.Auer:
>> > The other use case wants to follow the commits of that other
>> > submodule without checking the corresponding gitlinks into the
>> > superproject. But wouldn't it also make sense here to define
>> > actually a mapping in the .gitmodule that says "if the branch
>> > 'develop' is checkedout in the supermodule then with every
>> > submodule update automatically pull the newest 'unstable' commit
>> > from the submodule"? Or for "master" follow "stable" or for the
>> > "maint" branch follow updates in the "bugfixes" branch.
>> >
>> > For example
>> >
>> > [submodule "commonlib"] update = heads develop:unstable
>> > master:stable maint:bugfixes
>>
>> Having that configured with "branch=unstable", "branch=stable" etc.
>> in .gitmodules on the superprojects branches would be simpler and
>> achieve the same functionality.
>
> Yes, this has been my first thought also, but there is also a good point to keep the .gitmodules stable, or you always have to change the file when branches change their names. So when the maint branch of version 1.3 become an archive branch and the new maint is on 1.4, which was the master before then you have to change the .gitmodules on these branches. I.e. .gitmodules of 1.4 have "stable" and must have "bugfixes" now and .gitmodules of 1.3 has "bugfixes" and must remove the floating completely. I'm not sure that this can always be solved with "easy" merging and therefore it is probably not really simpler, when you have to do this for every new release. Or what do you think?
I never rename branches, so I do not concur ;-) And I think the
.gitmodules file could benefit from a special merge driver being
aware of the format and some subtleties (like not just adding a
"branch" setting but rather creating a merge conflict) anyways.
So I'd prefer to keep it simple and just use the .gitmodules we
already have which can be different in different branches.
>> > So whenever a defined branch is checked out in the superproject
>> > the mapped branch will be checked out in the submodule ("new"
>> > commit), but if a (e.g. tagged) commit is checked out ("old"
>> > commit) then the gitlink in the superproject is used to check out
>> > the referenced commit in the submodule.
>>
>> I think checkout should only use the submodule commit recorded in the
>> superproject and a subsequent "git submodule update" should be needed
>> to update the submodule to tip. Otherwise you record SHA-1 but still
>> won't be able to bisect ...
>
> bisect would leave the branch and therefore uses the recorded SHA1 for the submodule checkout instead of the tip. "follow-the-tip" should only work if the superproject follows the tip.
If you follow a tip there won't be any new SHA-1s recorded during
that following so you could not do a bisect and expect the submodule
to be what the developer had when doing the commits, no?
> If you configure auto-update on checkout you would not expect that a separate git submodule update has a different behavior.
Sure you do, when auto-update on checkout is active "git submodule update"
becomes a no-op for the exact submodule model, as "git checkout" will do
all the work "git submodule update" did before.
>> > In http://thread.gmane.org/gmane.comp.version-control.git/183837
>> > was discussed whether the gitlink in the superproject should be
>> > set to all-zero if updates follow the tip or maybe use the SHA1 of
>> > the commit when the submodule was added. I think the gitlink should
>> > be updated everytime when a new commit in the superproject is
>> > created.
>>
>> Nope, only when "git submodule update" is run. Otherwise you'll
>> spray the history with submodule updates totally unrelated to the
>> commits in the superproject, which is rather confusing.
>
> Of course, committing a new version to the superproject should not trigger pulling in a new version for the submodule or an automatic jump to the tip of the submodule. I just meant a normal manual "commit -a" behavior. Putting a 0{40} hash in the gitlink or only the hash of the submodule, when it first was added would be a special treatment that is neither needed nor wanted.
I don't get that, what SHA-1 do you want to put into the gitlink?
I understand that floating is not about updating the SHA-1 for the
submodule each commit, right?
^ permalink raw reply
* Re: [PATCH 1/2] t5540: test DAV push with authentication
From: Sebastian Schuberth @ 2011-12-13 21:28 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Stefan Naewe, Eric, git, msysgit
In-Reply-To: <20111213201704.GA12072@sigill.intra.peff.net>
On Tue, Dec 13, 2011 at 21:17, Jeff King <peff@peff.net> wrote:
> We don't currently test this case at all, and instead just
> test the DAV mechanism over an unauthenticated push. That
> isn't very realistic, as most people will want to
> authenticate pushes.
Thanks for adding this, Peff!
--
Sebastian Schuberth
^ permalink raw reply
* Re: [PATCH] Revert "http: don't always prompt for password"
From: Junio C Hamano @ 2011-12-13 21:25 UTC (permalink / raw)
To: Jeff King; +Cc: Stefan Naewe, Sebastian Schuberth, Eric, git, msysgit
In-Reply-To: <20111213202508.GA12187@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> @@ -844,7 +846,7 @@ static int http_request(const char *url, void *result, int target, int options)
> else if (missing_target(&results))
> ret = HTTP_MISSING_TARGET;
> else if (results.http_code == 401) {
> - if (user_name && user_pass) {
> + if (user_name) {
> ret = HTTP_NOAUTH;
> } else {
> /*
In the credential series, this is where we declare the given credential is
to be rejected (if we have both username and password), or ask them to be
filled by calling credential_fill(), so I think the code in the credential
series does not need this revert. Right?
> @@ -853,8 +855,7 @@ static int http_request(const char *url, void *result, int target, int options)
> * but that is non-portable. Using git_getpass() can at least be stubbed
> * on other platforms with a different implementation if/when necessary.
> */
> - if (!user_name)
> - user_name = xstrdup(git_getpass_with_description("Username", description));
> + user_name = xstrdup(git_getpass_with_description("Username", description));
> init_curl_http_auth(slot->curl);
> ret = HTTP_REAUTH;
> }
So is this one.
^ permalink raw reply
* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Jens Lehmann @ 2011-12-13 21:19 UTC (permalink / raw)
To: Marc Branchaud
Cc: Phil Hord, Leif Gruenwoldt, Andreas T.Auer, Junio C Hamano, git
In-Reply-To: <4EE770D0.5080702@xiplink.com>
Am 13.12.2011 16:35, schrieb Marc Branchaud:
> I'd prefer to have floating be explicitly configured on a per-branch (or
> per-branch-glob) basis. So in addition to what Jens described yesterday [1]
> to configure an individual submodule's floating branch, I suggest there also
> be a new section in the .gitmodules file for configuring the super-repo's
> floating branches, e.g.
>
> [super]
> floaters = refs/heads/master refs/heads/dev*
>
> [submodule "Sub1"]
> path = foo/bar
> branch = maint
> url = ...
>
> [submodule "Sub2"]
> path = other/place
> url = ...
Hmm, but you can have different .gitmodules files in different branches of
the superproject, no? Why not just have the "branch = maint" setting for
"Sub1" in the master and the dev branches .gitmodules file and drop it in
the other branches?
> This would mean that whenever the super-repo checks out either the "master"
> branch or a branch whose name starts with "dev" (assuming recursive checkouts
> are on):
>
> * The Sub1 submodule automatically checks out the tip of its
> "maint" branch.
>
> * The Sub2 submodule (lacking a "branch" variable) would not float
> and would check out the commit recorded in the super-repo.
>
> A super-repo recursive-checkout that doesn't match a floaters pattern would
> work in the regular, non-floating way.
Which would just work with my proposal too if git would honor the
.gitmodules file of the currently checked out branch.
> [1] http://article.gmane.org/gmane.comp.version-control.git/186969
^ permalink raw reply
* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Jens Lehmann @ 2011-12-13 21:09 UTC (permalink / raw)
To: Phil Hord; +Cc: Junio C Hamano, Leif Gruenwoldt, git
In-Reply-To: <CABURp0pPqpkWXdC+97wR8HZeX=Nbi0bn-3ji+k9LQnj0kFjCnQ@mail.gmail.com>
Am 13.12.2011 15:17, schrieb Phil Hord:
> On Mon, Dec 12, 2011 at 2:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
> [...]
>> Distro package dependency tracking is a poor analogy for many reasons, but
>> I'll only touch a few.
> [...]
>> Naively, one might think that two branches, branch-1.0 and branch-2.0, can
>> be defined in the repository of L, tell somebody (like "superproject that
>> covers all the packages in the distro") that A wants branch-1.0 and B
>> wants branch-2.0 of L respectively, to emulate this, but if one thinks
>> further, one would realize that it is insufficient. For one thing, it is
>> unclear what should happen when both A and B are asked to be checked out,
>> but more importantly, in dependency requirements on real distro packaging,
>> the application C could say "I want v1.0 API but v1.4 is broken and not
>> compatible with me", which won't fit on the two-branches model. A
>> workaround to add more branches to L could be devised but any workaround
>> cannot be a good solution that allows a random application C among 47
>> others to dictate how the branch structure of L project should look like.
>>
>> Fortunately, the dependency management is a solved problem by distro
>> package management and build systems, and they do so without using
>> anything from submodules. There is no point reinventing these logic in git
>> submodules and emulating poorly.
>>
>> The only remotely plausible analogy around distro packaging would be a
>> superproject full of all the packages in a distro as its submodules, and
>> records exact versions of each and every package that goes on a release
>> CD (or DVD). In that case, you do want to have a central registry that
>> records what exact version of each package is used to cut the CD and the
>> mother of all modules superproject could be one way to implement it. But
>> that is not an example of floating, but is a direct opposite.
>>
>> This exchange convinced me further that anybody who wishes to use
>> "floating" is better off either by doing one or both of the following:
>>
>> - using "exact" but not updating religiously, as the interdepency
>> requirement in their project is not strict; or
>>
>> - not using submodules at all, but merely keeping these unrelated A, B, C
>> and L as standalone repositories next to each other in the directory
>> structure.
>
> My interdependency requirements are not so cut-and-dry. We use
> submodules to isolate controlled regions of code. We may need to
> share our project with a contractor who is allowed to see code
> pertaining to "vendorA" but not that for "vendorB" or "VendorN". But
> our in-house developers want to have all the vendor code in one place
> for convenient integration. Submodules do this nicely for us. We can
> give the contractor just the main modules and the VendorA modules and
> he'll be fine. In-house devs get all the submodules (using the
> vendor-ALL superproject).
>
> But this necessarily means there is too much coupling for comfort
> between our submodules. For example, when an API changes in the main
> submodule, each of the vendor submodules is affected because they each
> implement that API in a custom method. Some of those vendor modules
> belong to different people. Submodule synchronization becomes a real
> chore.
Hmm, maybe having vendor-specific branches in the superproject would
help here. But that is hard to tell without knowing more details about
your setup. But I suspect your vendor-ALL superproject is exactly the
right spot to deal with these kind of problems (and if that isn't easy
that might be a result of the difficulty of the problem you are trying
to solve here, keeping different vendors in sync with your API ;-).
> Floating would help, I think. Instead I do this:
>
> git pull origin topic_foo && git submodule foreach 'git pull origin topic_foo'
>
> git submodule foreach 'git push origin topic_foo' && git push origin topic_foo
This sounds to me like you would need the "--recurse-submodules" option
implemented for "git pull" and "git push", no? And I miss to see how
floating would help when the tips of some submodules are not ready to
work with other submodules tips ...
> But not all my developers are git-gurus yet, and they sometimes mess
> up their ad hoc scripts or miss important changes they forgot to push
> in one submodule or another.
Sure, even though current git should help you some by showing changes
in the submodules.
> Or worse, their pull or push fails and
> they can't see the problem for all the noise. So they email it to me.
We circumvent that by not pulling, but fetching and merging in the
submodule first and after that in the superproject. You have much more
control about what is going wrong where (and can have more
git-experienced people help with - or even do - the merges).
> On my git server, I have a hook that automatically propagates each
> push to "master" from the submodules into the superproject. But this
> is tedious and limited. And it relies on a centralized server.
But for closely related stuff that is a good option. Our continuous
integration server shows us quite some breakage between submodules
before they hit a superproject, which is really helpful.
> You may say this itch is all in my head, but it sure seems real to me.
This definitely is a real problem. Lets see how far git can help you
here...
^ permalink raw reply
* Re: [PATCH] Revert "http: don't always prompt for password"
From: Junio C Hamano @ 2011-12-13 21:09 UTC (permalink / raw)
To: Jeff King; +Cc: Stefan Naewe, Sebastian Schuberth, Eric, git, msysgit
In-Reply-To: <20111213202508.GA12187@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Doing (3) is obviously the easiest thing. And given the complexity of
> the other two solutions, I think it makes sense to revert 986bbc08
> (i.e., apply this patch), ship a working v1.7.8.1, and then look at
> doing one of the other two solutions for v1.7.9.
Or just let the "dumb HTTP" die.
I thought push over DAV has long been dead; is anybody using it for real?
^ permalink raw reply
* [PATCH] Revert "http: don't always prompt for password"
From: Jeff King @ 2011-12-13 20:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Naewe, Sebastian Schuberth, Eric, git, msysgit
In-Reply-To: <20111213201704.GA12072@sigill.intra.peff.net>
This reverts commit 986bbc0842334f0e07731fa37f2a55d2930a5b8c.
The rationale for that commit relied on the fact that asking
for the password up-front was merely an optimization,
because git will notice an HTTP 401 and prompt for the
password. However, that is only true for smart-http, and for
dumb fetching. Dumb push over DAV does not have this
feature; as a result, authenticated push-over-DAV does not
work at all, as it never prompts the user for a password.
Signed-off-by: Jeff King <peff@peff.net>
---
We need to deal with this regression for v1.7.8.1, I think.
There are basically three options for fixing it:
1. Teach http-push the same retry-after-401 trick that the rest of the
http code knows.
2. Refactor the retry-after-401 logic from http.c into a common
function that http-push can build on top of.
3. Revert 986bbc08 and leave it alone; it only hurts .netrc users,
there's a reasonable workaround (don't put the user in the URL) and
hopefully those people will convert to using better storage via
credential helper once it is available.
I looked at doing (1), but my first attempt[1] didn't quite work. So
it's not a huge amount of code, but it's annoyingly non-trivial. And as
a long-term solution, it's just making hack-y code hackier.
Doing (2) would be the best solution, but it's going to require some
pretty major surgery to http.c and http-push.c. I'll take a look, but if
it gets too complex, it may simply not be worth it (now that smart-http
is available, I would hope that push-over-DAV is slowly going away).
Doing (3) is obviously the easiest thing. And given the complexity of
the other two solutions, I think it makes sense to revert 986bbc08
(i.e., apply this patch), ship a working v1.7.8.1, and then look at
doing one of the other two solutions for v1.7.9.
[1] http://article.gmane.org/gmane.comp.version-control.msysgit/14153
http.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/http.c b/http.c
index 008ad72..a4bc770 100644
--- a/http.c
+++ b/http.c
@@ -279,6 +279,8 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
#endif
+ init_curl_http_auth(result);
+
if (ssl_cert != NULL)
curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
if (has_cert_password())
@@ -844,7 +846,7 @@ static int http_request(const char *url, void *result, int target, int options)
else if (missing_target(&results))
ret = HTTP_MISSING_TARGET;
else if (results.http_code == 401) {
- if (user_name && user_pass) {
+ if (user_name) {
ret = HTTP_NOAUTH;
} else {
/*
@@ -853,8 +855,7 @@ static int http_request(const char *url, void *result, int target, int options)
* but that is non-portable. Using git_getpass() can at least be stubbed
* on other platforms with a different implementation if/when necessary.
*/
- if (!user_name)
- user_name = xstrdup(git_getpass_with_description("Username", description));
+ user_name = xstrdup(git_getpass_with_description("Username", description));
init_curl_http_auth(slot->curl);
ret = HTTP_REAUTH;
}
--
1.7.8.17.gfd3524
^ permalink raw reply related
* [PATCH 1/2] t5540: test DAV push with authentication
From: Jeff King @ 2011-12-13 20:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Naewe, Sebastian Schuberth, Eric, git, msysgit
We don't currently test this case at all, and instead just
test the DAV mechanism over an unauthenticated push. That
isn't very realistic, as most people will want to
authenticate pushes.
Two of the tests expect_failure as they reveal bugs:
1. Pushing without a username in the URL fails to ask for
credentials when we get an HTTP 401. This has always
been the case, but it would be nice if it worked like
smart-http.
2. Pushing with a username fails to ask for the password
since 986bbc0 (http: don't always prompt for password,
2011-11-04). This is a severe regression in v1.7.8, as
authenticated push-over-DAV is now totally unusable
unless you have credentials in your .netrc.
Signed-off-by: Jeff King <peff@peff.net>
---
Nobody has mentioned the regression on git@vger yet, but there are two
threads already on msysgit:
http://thread.gmane.org/gmane.comp.version-control.msysgit/14138
http://thread.gmane.org/gmane.comp.version-control.msysgit/14161
t/lib-httpd/apache.conf | 3 +++
t/t5540-http-push.sh | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 37 insertions(+), 0 deletions(-)
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 0a4cdfa..3c12b05 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -92,6 +92,9 @@ SSLEngine On
<Location /dumb/>
Dav on
</Location>
+ <Location /auth/dumb>
+ Dav on
+ </Location>
</IfDefine>
<IfDefine SVN>
diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index 64767d8..3300227 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh
@@ -40,6 +40,22 @@ test_expect_success 'setup remote repository' '
mv test_repo.git "$HTTPD_DOCUMENT_ROOT_PATH"
'
+test_expect_success 'create password-protected repository' '
+ mkdir -p "$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb" &&
+ cp -Rf "$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git" \
+ "$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/test_repo.git"
+'
+
+test_expect_success 'setup askpass helper' '
+ cat >askpass <<-\EOF &&
+ #!/bin/sh
+ echo user@host
+ EOF
+ chmod +x askpass &&
+ GIT_ASKPASS="$PWD/askpass" &&
+ export GIT_ASKPASS
+'
+
test_expect_success 'clone remote repository' '
cd "$ROOT_PATH" &&
git clone $HTTPD_URL/dumb/test_repo.git test_repo_clone
@@ -144,6 +160,24 @@ test_expect_success 'PUT and MOVE sends object to URLs with SHA-1 hash suffix' '
test_http_push_nonff "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \
"$ROOT_PATH"/test_repo_clone master
+test_expect_failure 'push to password-protected repository (user in URL)' '
+ test_commit pw-user &&
+ git push "$HTTPD_URL_USER/auth/dumb/test_repo.git" HEAD &&
+ git rev-parse --verify HEAD >expect &&
+ git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/test_repo.git" \
+ rev-parse --verify HEAD >actual &&
+ test_cmp expect actual
+'
+
+test_expect_failure 'push to password-protected repository (no user in URL)' '
+ test_commit pw-nouser &&
+ git push "$HTTPD_URL/auth/dumb/test_repo.git" HEAD &&
+ git rev-parse --verify HEAD >expect &&
+ git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/test_repo.git" \
+ rev-parse --verify HEAD >actual &&
+ test_cmp expect actual
+'
+
stop_httpd
test_done
--
1.7.8.17.gfd3524
^ permalink raw reply related
* [PATCH 2/6] add_extra_ref(): remove flag argument
From: mhagger @ 2011-12-13 20:06 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1323806811-5798-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
The argument was always set to 0 (and other values do not make sense)
so remove the argument.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/clone.c | 4 ++--
builtin/receive-pack.c | 2 +-
refs.c | 4 ++--
refs.h | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index efe8b6c..5035767 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -252,7 +252,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)
transport = transport_get(remote, ref_git);
for (extra = transport_get_remote_refs(transport); extra;
extra = extra->next)
- add_extra_ref(extra->name, extra->old_sha1, 0);
+ add_extra_ref(extra->name, extra->old_sha1);
transport_disconnect(transport);
free(ref_git);
@@ -441,7 +441,7 @@ static void write_remote_refs(const struct ref *local_refs)
for (r = local_refs; r; r = r->next) {
if (!r->peer_ref)
continue;
- add_extra_ref(r->peer_ref->name, r->old_sha1, 0);
+ add_extra_ref(r->peer_ref->name, r->old_sha1);
}
pack_refs(PACK_REFS_ALL);
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index b6d957c..e3b46ce 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -872,7 +872,7 @@ static int delete_only(struct command *commands)
static void add_one_alternate_sha1(const unsigned char sha1[20], void *unused)
{
- add_extra_ref(".have", sha1, 0);
+ add_extra_ref(".have", sha1);
}
static void collect_one_alternate_ref(const struct ref *ref, void *data)
diff --git a/refs.c b/refs.c
index f5cb297..6115487 100644
--- a/refs.c
+++ b/refs.c
@@ -248,9 +248,9 @@ static void read_packed_refs(FILE *f, struct ref_array *array)
sort_ref_array(array);
}
-void add_extra_ref(const char *name, const unsigned char *sha1, int flag)
+void add_extra_ref(const char *name, const unsigned char *sha1)
{
- add_ref(name, sha1, flag, 0, &extra_refs, NULL);
+ add_ref(name, sha1, 0, 0, &extra_refs, NULL);
}
void clear_extra_refs(void)
diff --git a/refs.h b/refs.h
index 3fd5536..39bb289 100644
--- a/refs.h
+++ b/refs.h
@@ -56,7 +56,7 @@ extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refn
* called. Only extra refs added before for_each_ref() is called will
* be listed on a given call of for_each_ref().
*/
-extern void add_extra_ref(const char *refname, const unsigned char *sha1, int flags);
+extern void add_extra_ref(const char *refname, const unsigned char *sha1);
extern void clear_extra_refs(void);
extern int ref_exists(const char *);
--
1.7.8
^ permalink raw reply related
* [PATCH 4/6] Store extra_refs in a separate data structure
From: mhagger @ 2011-12-13 20:06 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1323806811-5798-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Extra references really don't have much to do with real references,
and we want to change how real references are handled. So hold extra
references in a separate data structure.
Since extra references cannot be broken, don't store flags with them
and don't pass the flags argument (a different "flags"!) to
do_for_each_extra_ref().
Finally, current_ref is not useful while iterating through the
extra_refs. In fact, peel_ref() doesn't even work for extra refs
because they cannot be peeled themselves, and moreover they might
coincidentally have the same name as a "real" reference. So
explicitly set current_ref to NULL while iterating over extra_refs to
avoid confusion.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Strictly speaking, this patch changes how an extra reference with an
invalid SHA1 would be treated. In the old code, it would be skipped
over (assuming DO_FOR_EACH_INCLUDE_BROKEN is not set). In the new
code the SHA1 is not checked. However, from my understanding of how
extra_refs are used, they should never have invalid SHA1s and so the
old test is superfluous.
refs.c | 38 ++++++++++++++++++++++++++++----------
1 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index 268bda9..843c530 100644
--- a/refs.c
+++ b/refs.c
@@ -146,7 +146,11 @@ static struct ref_cache {
static struct ref_entry *current_ref;
-static struct ref_array extra_refs;
+static struct extra_ref {
+ struct extra_ref *next;
+ unsigned char sha1[20];
+ char name[FLEX_ARRAY];
+} *extra_refs;
static void free_ref_array(struct ref_array *array)
{
@@ -250,12 +254,21 @@ static void read_packed_refs(FILE *f, struct ref_array *array)
void add_extra_ref(const char *name, const unsigned char *sha1)
{
- add_ref(name, sha1, 0, 0, &extra_refs, NULL);
+ int len = strlen(name) + 1;
+ struct extra_ref *entry = xmalloc(sizeof(struct extra_ref) + len);
+ hashcpy(entry->sha1, sha1);
+ memcpy(entry->name, name, len);
+ entry->next = extra_refs;
+ extra_refs = entry;
}
void clear_extra_refs(void)
{
- free_ref_array(&extra_refs);
+ while (extra_refs) {
+ struct extra_ref *entry = extra_refs;
+ extra_refs = entry->next;
+ free(entry);
+ }
}
static struct ref_array *get_packed_refs(const char *submodule)
@@ -694,13 +707,18 @@ fallback:
}
static int do_for_each_extra_ref(const char *base, each_ref_fn fn,
- int trim, int flags, void *cb_data)
+ int trim, void *cb_data)
{
- int i;
- for (i = 0; i < extra_refs.nr; i++) {
- int retval = do_one_ref(base, fn, trim, flags, cb_data, extra_refs.refs[i]);
- if (retval)
- return retval;
+ struct extra_ref *extra;
+
+ current_ref = NULL;
+ for (extra = extra_refs; extra; extra = extra->next) {
+ if (!prefixcmp(extra->name, base)) {
+ int retval = fn(extra->name + trim, extra->sha1,
+ 0, cb_data);
+ if (retval)
+ return retval;
+ }
}
return 0;
}
@@ -712,7 +730,7 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn
struct ref_array *packed = get_packed_refs(submodule);
struct ref_array *loose = get_loose_refs(submodule);
- retval = do_for_each_extra_ref(base, fn, trim, flags, cb_data);
+ retval = do_for_each_extra_ref(base, fn, trim, cb_data);
if (retval)
goto end_each;
--
1.7.8
^ permalink raw reply related
* [PATCH 6/6] do_for_each_extra_ref(): simplify signature
From: mhagger @ 2011-12-13 20:06 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1323806811-5798-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Now that do_for_each_extra_ref() is only called from one place, it can
be less general.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 14 +++++---------
1 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index f52d8b5..4b2ba3f 100644
--- a/refs.c
+++ b/refs.c
@@ -706,19 +706,15 @@ fallback:
return -1;
}
-static int do_for_each_extra_ref(const char *base, each_ref_fn fn,
- int trim, void *cb_data)
+static int do_for_each_extra_ref(each_ref_fn fn, void *cb_data)
{
struct extra_ref *extra;
current_ref = NULL;
for (extra = extra_refs; extra; extra = extra->next) {
- if (!prefixcmp(extra->name, base)) {
- int retval = fn(extra->name + trim, extra->sha1,
- 0, cb_data);
- if (retval)
- return retval;
- }
+ int retval = fn(extra->name, extra->sha1, 0, cb_data);
+ if (retval)
+ return retval;
}
return 0;
}
@@ -794,7 +790,7 @@ int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
int for_each_ref(each_ref_fn fn, void *cb_data)
{
- int retval = do_for_each_extra_ref("", fn, 0, cb_data);
+ int retval = do_for_each_extra_ref(fn, cb_data);
if (retval)
return retval;
return do_for_each_ref(NULL, "", fn, 0, 0, cb_data);
--
1.7.8
^ permalink raw reply related
* [PATCH 3/6] Extract a function do_for_each_extra_ref()
From: mhagger @ 2011-12-13 20:06 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1323806811-5798-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
We want to hold the extra refs at arms length.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 21 ++++++++++++++++-----
1 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index 6115487..268bda9 100644
--- a/refs.c
+++ b/refs.c
@@ -693,17 +693,28 @@ fallback:
return -1;
}
+static int do_for_each_extra_ref(const char *base, each_ref_fn fn,
+ int trim, int flags, void *cb_data)
+{
+ int i;
+ for (i = 0; i < extra_refs.nr; i++) {
+ int retval = do_one_ref(base, fn, trim, flags, cb_data, extra_refs.refs[i]);
+ if (retval)
+ return retval;
+ }
+ return 0;
+}
+
static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn fn,
int trim, int flags, void *cb_data)
{
- int retval = 0, i, p = 0, l = 0;
+ int retval = 0, p = 0, l = 0;
struct ref_array *packed = get_packed_refs(submodule);
struct ref_array *loose = get_loose_refs(submodule);
- struct ref_array *extra = &extra_refs;
-
- for (i = 0; i < extra->nr; i++)
- retval = do_one_ref(base, fn, trim, flags, cb_data, extra->refs[i]);
+ retval = do_for_each_extra_ref(base, fn, trim, flags, cb_data);
+ if (retval)
+ goto end_each;
while (p < packed->nr && l < loose->nr) {
struct ref_entry *entry;
--
1.7.8
^ permalink raw reply related
* [PATCH 5/6] Omit extra_refs except when iterating using for_each_ref()
From: mhagger @ 2011-12-13 20:06 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1323806811-5798-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
According to Junio, the only reference iteration function that needs
to include the extra refs is for_each_ref(). So call
do_for_each_extra_ref() directly from there instead of from
do_for_each_ref().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 843c530..f52d8b5 100644
--- a/refs.c
+++ b/refs.c
@@ -730,10 +730,6 @@ static int do_for_each_ref(const char *submodule, const char *base, each_ref_fn
struct ref_array *packed = get_packed_refs(submodule);
struct ref_array *loose = get_loose_refs(submodule);
- retval = do_for_each_extra_ref(base, fn, trim, cb_data);
- if (retval)
- goto end_each;
-
while (p < packed->nr && l < loose->nr) {
struct ref_entry *entry;
int cmp = strcmp(packed->refs[p]->name, loose->refs[l]->name);
@@ -798,6 +794,9 @@ int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
int for_each_ref(each_ref_fn fn, void *cb_data)
{
+ int retval = do_for_each_extra_ref("", fn, 0, cb_data);
+ if (retval)
+ return retval;
return do_for_each_ref(NULL, "", fn, 0, 0, cb_data);
}
--
1.7.8
^ permalink raw reply related
* [PATCH 0/6] Handle extra_refs separately from ref_caches
From: mhagger @ 2011-12-13 20:06 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
Extra refs don't have much to do with real references, and in fact
they have to be handled differently. For example, they do not support
flags, they might not have unique names (indeed, the names are rather
meaningless), and they are only ever iterated over, never looked up.
So seemingly innocent things that one might want to do with real
references, like check for conflicting duplicates, must not be done
for extra refs.
This patch series creates a new linked-list data structure for the
extra refs, separates iteration over the extra refs into a new
function, and changes a test to actually create multiple extra refs
with the same name.
This patch series applies on top of master. If this approach is
selected, then the ref-api-D series will have to be rebased on top of
it and touched up to avoid the problems that it has with duplicate
extra refs.
By the way, I have been carrying around the CC list of this email for
quite a while. If you are tired of being spammed with my patch
series, send me a private email and I will be happy to remove you from
future mailings.
Michael Haggerty (6):
t5519: push two branches to alternate repo
add_extra_ref(): remove flag argument
Extract a function do_for_each_extra_ref()
Store extra_refs in a separate data structure
Omit extra_refs except when iterating using for_each_ref()
do_for_each_extra_ref(): simplify signature
builtin/clone.c | 4 ++--
builtin/receive-pack.c | 2 +-
refs.c | 44 ++++++++++++++++++++++++++++++++++----------
refs.h | 2 +-
t/t5519-push-alternates.sh | 10 +++++++++-
5 files changed, 47 insertions(+), 15 deletions(-)
--
1.7.8
^ permalink raw reply
* [PATCH 1/6] t5519: push two branches to alternate repo
From: mhagger @ 2011-12-13 20:06 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1323806811-5798-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Since each branch in the alternate repo results in an "extra_ref"
named ".have", pushing two of them results in two extra_refs with the
same name. This change to the test therefore makes sure that we can
handle extra_refs names that are not unique.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
I'm not sure how well this change fits into the other things that the
test wants to do, but it triggers the failure mode in ref-api-D v2
that was predicted by Junio.
t/t5519-push-alternates.sh | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/t/t5519-push-alternates.sh b/t/t5519-push-alternates.sh
index c00c9b0..315f65d 100755
--- a/t/t5519-push-alternates.sh
+++ b/t/t5519-push-alternates.sh
@@ -17,7 +17,15 @@ test_expect_success setup '
>file &&
git add . &&
git commit -m initial &&
- git push ../alice-pub master
+ git checkout -b foo &&
+ >file1 &&
+ git add . &&
+ git commit -m file1 &&
+ git checkout master &&
+ >file2 &&
+ git add . &&
+ git commit -m file2 &&
+ git push ../alice-pub master foo
) &&
# Project Bob is a fork of project Alice
--
1.7.8
^ permalink raw reply related
* Re: [PATCH 2/2] Makefile: optionally exclude code that needs Unix sockets
From: Johannes Sixt @ 2011-12-13 20:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vzkexwb7m.fsf@alter.siamese.dyndns.org>
Am 13.12.2011 01:45, schrieb Junio C Hamano:
> I'll queue a single patch that is a squash between 2/2 and Peff's test
> updates between "credentials: add "cache" helper" and "strbuf: add
> strbuf_add*_urlencode" in the series.
Thanks. The resulting series builds fine on Windows and passes/skips the
new tests in the expected manner.
-- Hannes
^ permalink raw reply
* Re: [PATCH/RFC 1/2] wrapper: supply xsetenv
From: Andreas Schwab @ 2011-12-13 19:21 UTC (permalink / raw)
To: kusmabite; +Cc: Junio C Hamano, Jeff King, git
In-Reply-To: <CABPQNSZ_r4u-yXtEGw8duZyoN3SdF5p+7vabz2qqS161kgkHWQ@mail.gmail.com>
Erik Faye-Lund <kusmabite@gmail.com> writes:
> Older Linux kernels maxed out at 128 kB.
But you wouldn't detect that at setenv time.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox