* Re: [RFC/PATCH] remote-curl: Obey passed URL
From: Jeff King @ 2011-10-06 13:37 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Shawn O. Pearce, Tay Ray Chuan
In-Reply-To: <20111006132500.GA1792@sigill.intra.peff.net>
On Thu, Oct 06, 2011 at 09:25:00AM -0400, Jeff King wrote:
> Your analysis is correct, but tweaking the remote object seems kind
> of ugly. I think a nicer solution would be to pass the URL in
> separately to http_init. Of the three existing callers:
Here's what that patch looks like. It's definitely an improvement and
fixes a real bug, so it may be worth applying. But I'm still going to
look into pushing the url examination closer to the point of use.
-- >8 --
Subject: [PATCH] http_init: accept separate URL parameter
The http_init function takes a "struct remote". Part of its
initialization procedure is to look at the remote's url and
grab some auth-related parameters. However, using the url
included in the remote is:
- wrong; the remote-curl helper may have a separate,
unrelated URL (e.g., from remote.*.pushurl). Looking at
the remote's configured url is incorrect.
- incomplete; http-fetch doesn't have a remote, so passes
NULL. So http_init never gets to see the URL we are
actually going to use.
- cumbersome; http-push has a similar problem to
http-fetch, but actually builds a fake remote just to
pass in the URL.
Instead, let's just add a separate URL parameter to
http_init, and all three callsites can pass in the
appropriate information.
Signed-off-by: Jeff King <peff@peff.net>
---
http-fetch.c | 2 +-
http-push.c | 10 +---------
http.c | 8 ++++----
http.h | 2 +-
remote-curl.c | 2 +-
5 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/http-fetch.c b/http-fetch.c
index 3af4c71..e341872 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -63,7 +63,7 @@ int main(int argc, const char **argv)
git_config(git_default_config, NULL);
- http_init(NULL);
+ http_init(NULL, url);
walker = get_http_walker(url);
walker->get_tree = get_tree;
walker->get_history = get_history;
diff --git a/http-push.c b/http-push.c
index 376331a..215b640 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1747,7 +1747,6 @@ int main(int argc, char **argv)
int i;
int new_refs;
struct ref *ref, *local_refs;
- struct remote *remote;
git_extract_argv0_path(argv[0]);
@@ -1821,14 +1820,7 @@ int main(int argc, char **argv)
memset(remote_dir_exists, -1, 256);
- /*
- * Create a minimum remote by hand to give to http_init(),
- * primarily to allow it to look at the URL.
- */
- remote = xcalloc(sizeof(*remote), 1);
- ALLOC_GROW(remote->url, remote->url_nr + 1, remote->url_alloc);
- remote->url[remote->url_nr++] = repo->url;
- http_init(remote);
+ http_init(NULL, repo->url);
#ifdef USE_CURL_MULTI
is_running_queue = 0;
diff --git a/http.c b/http.c
index b2ae8de..d9f9938 100644
--- a/http.c
+++ b/http.c
@@ -357,7 +357,7 @@ static void set_from_env(const char **var, const char *envname)
*var = val;
}
-void http_init(struct remote *remote)
+void http_init(struct remote *remote, const char *url)
{
char *low_speed_limit;
char *low_speed_time;
@@ -421,11 +421,11 @@ void http_init(struct remote *remote)
if (getenv("GIT_CURL_FTP_NO_EPSV"))
curl_ftp_no_epsv = 1;
- if (remote && remote->url && remote->url[0]) {
- http_auth_init(remote->url[0]);
+ if (url) {
+ http_auth_init(url);
if (!ssl_cert_password_required &&
getenv("GIT_SSL_CERT_PASSWORD_PROTECTED") &&
- !prefixcmp(remote->url[0], "https://"))
+ !prefixcmp(url, "https://"))
ssl_cert_password_required = 1;
}
diff --git a/http.h b/http.h
index 0bf8592..3c332a9 100644
--- a/http.h
+++ b/http.h
@@ -86,7 +86,7 @@ struct buffer {
extern void step_active_slots(void);
#endif
-extern void http_init(struct remote *remote);
+extern void http_init(struct remote *remote, const char *url);
extern void http_cleanup(void);
extern int data_received;
diff --git a/remote-curl.c b/remote-curl.c
index 69831e9..33d3d8c 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -852,7 +852,7 @@ int main(int argc, const char **argv)
url = strbuf_detach(&buf, NULL);
- http_init(remote);
+ http_init(remote, url);
do {
if (strbuf_getline(&buf, stdin, '\n') == EOF)
--
1.7.7.37.g0e376
^ permalink raw reply related
* Re: git-cherry-pick and author field in version 1.7.6.4
From: Nicolas Dichtel @ 2011-10-06 13:34 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111006132646.GB1792@sigill.intra.peff.net>
Le 06/10/2011 15:26, Jeff King a écrit :
> On Thu, Oct 06, 2011 at 02:37:02PM +0200, Nicolas Dichtel wrote:
>
>>>> Maybe it is related to the problem I've reported in another thread:
>>>> http://comments.gmane.org/gmane.comp.version-control.git/182853
>>>
>>> Possibly. That issue is about the commit that comes _after_ the
>>> cherry-pick, and in this instance, things are already wrong for you by
>>> the time the cherry-pick has completed.
>>>
>>> However, the problem has to do with leaving a stale state file in .git,
>>> so perhaps a previous partially-completed cherry-pick has left cruft in
>>> .git that is confusing this cherry-pick (i.e., I can't reproduce because
>>> it is being affected by something that happened before the commands
>>> above). So let's see what Jay comes up with for solving the other
>>> problem, and I suspect it may just fix this issue, too.
>> I think so too. Will wait.
>
> Since you can reproduce this so readily, and since you said it seems to
> work with older versions of git, you might try bisecting. There's a
> reasonable chance it will just end up at Jay's CHERRY_PICK_HEAD commit,
> but it might be worth doing.
I've try with another user on the same host and it works, so I end up to my
environment config ... and it's my fault: I've some variables set (GITPERLLIB
and GIT_EXEC_PATH) that point to an older git version.
Now it works, sorry for the noise!
The second pb, about the author is fixed too.
Regards,
Nicolas
^ permalink raw reply
* Re: Merge seems to overwrite unstaged local changes
From: Sebastian Schuberth @ 2011-10-06 13:27 UTC (permalink / raw)
Cc: Junio C Hamano, git
In-Reply-To: <CAHGBnuNrhtyq1tfok3p9YHAVbfo9T7BO3ZOUy+8YvNE9Mmhjhg@mail.gmail.com>
On 29.09.2011 15:07, Sebastian Schuberth wrote:
>> There recently have been quite a change in merge-recursive implementation
>> and it would be really nice if you can try this again with the tip of
>> 'master' before 1.7.7 final ships.
>
> The unstaged changes do not seem to get lost during the merge anymore
> when using git version 1.7.7.rc3.4.g8d714 on Linux. I guess that
> somewhat confirms that there's a bug in git< 1.7.7. I'll write a word
> of warning to our in-house git users that they should always commit
> before merging ...
It seems I'm not the only one who lost code due to this bug. For a more
detailed analysis see this blog post:
http://benno.id.au/blog/2011/10/01/git-recursive-merge-broken
As it turns out, my use case also involves a rename of the file in which
changes were lost. And just like for the blog's author it somewhat
concerns me and shakes my confidence in Git for how long this severe bug
slipped through undetected.
--
Sebastian Schuberth
^ permalink raw reply
* Re: git-cherry-pick and author field in version 1.7.6.4
From: Jeff King @ 2011-10-06 13:26 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: git
In-Reply-To: <4E8DA0EE.50208@6wind.com>
On Thu, Oct 06, 2011 at 02:37:02PM +0200, Nicolas Dichtel wrote:
> >>Maybe it is related to the problem I've reported in another thread:
> >>http://comments.gmane.org/gmane.comp.version-control.git/182853
> >
> >Possibly. That issue is about the commit that comes _after_ the
> >cherry-pick, and in this instance, things are already wrong for you by
> >the time the cherry-pick has completed.
> >
> >However, the problem has to do with leaving a stale state file in .git,
> >so perhaps a previous partially-completed cherry-pick has left cruft in
> >.git that is confusing this cherry-pick (i.e., I can't reproduce because
> >it is being affected by something that happened before the commands
> >above). So let's see what Jay comes up with for solving the other
> >problem, and I suspect it may just fix this issue, too.
> I think so too. Will wait.
Since you can reproduce this so readily, and since you said it seems to
work with older versions of git, you might try bisecting. There's a
reasonable chance it will just end up at Jay's CHERRY_PICK_HEAD commit,
but it might be worth doing.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] remote-curl: Obey passed URL
From: Jeff King @ 2011-10-06 13:25 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Shawn O. Pearce, Tay Ray Chuan
In-Reply-To: <2f1eccfa3fa9e732e9bea344fd69dfd9b16697a9.1317906388.git.git@drmicha.warpmail.net>
On Thu, Oct 06, 2011 at 03:15:59PM +0200, Michael J Gruber wrote:
> When the curl remote helper is called, e.g., as
>
> git-remote-https foo https://john@doe.com
>
> it looks up remote.foo.url rather than taking the provided url, at least
> as far as http_init() is concerned (authentication). (It does use the
> provided url at later stages.)
> [...]
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -846,6 +846,7 @@ int main(int argc, const char **argv)
>
> if (argc > 2) {
> end_url_with_slash(&buf, argv[2]);
> + remote->url[0] = xstrdup(argv[2]);
> } else {
> end_url_with_slash(&buf, remote->url[0]);
> }
Your analysis is correct, but tweaking the remote object seems kind
of ugly. I think a nicer solution would be to pass the URL in
separately to http_init. Of the three existing callers:
1. http-fetch.c just passes a NULL remote. Which means we don't even
look at the URL at all for grabbing the auth information. Passing
the URL would be an improvement.
2. http-push.c creates a fake remote just to stick the URL in. That
ugly code could just go away.
3. remote-curl.c needs to pass in the alternate URL anyway, as you
described.
So it seems like all callsites would benefit.
That being said, I wonder if http_init is the right place to pull the
auth information out. It's where we've always done it, and it makes
sense if you are hitting one base URL. But what happens if we get a
redirect to some other site? Shouldn't we be deciding at the time of
making the request what the context of the http request is?
And then http_init can stop caring entirely what the URL is.
-Peff
^ permalink raw reply
* Re: git-cherry-pick and git-commit --amend in version 1.7.6.4
From: Nicolas Dichtel @ 2011-10-06 13:22 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <CAG+J_DzY6oW3CgCPDhD81Eue1Ygh+3pR7Q_NZEhauH_qkyUwqQ@mail.gmail.com>
Le 06/10/2011 15:09, Jay Soffian a écrit :
> On Thu, Oct 6, 2011 at 3:37 AM, Nicolas Dichtel
> <nicolas.dichtel@6wind.com> wrote:
>> # ls .git
>> branches COMMIT_EDITMSG config description FETCH_HEAD HEAD hooks
>> index info logs objects ORIG_HEAD packed-refs refs
>
> No CHERRY_PICK_HEAD, so far so good.
>
>> # git cherry-pick 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
>> [dev 4cca2c2] drivers/net/usb/asix.c: Fix unaligned accesses
>> 1 files changed, 33 insertions(+), 1 deletions(-)
>
> cherry-pick completes successfully.
>
>> # ls .git
>> branches CHERRY_PICK_HEAD COMMIT_EDITMSG config description FETCH_HEAD
>> HEAD hooks index info logs objects ORIG_HEAD packed-refs refs
>
> This is bad. CHERRY_PICK_HEAD should only exist if the cherry-pick failed.
>
> I really don't know what could cause this. Possibly a hook in your repo?
No hooks:
# ls .git/hooks/
applypatch-msg.sample post-commit.sample post-update.sample
pre-commit.sample pre-rebase.sample
commit-msg.sample post-receive.sample pre-applypatch.sample
prepare-commit-msg.sample update.sample
>
> Using "GIT_TRACE=1 git cherry-pick
> 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9" will tell you whether git is
> running any hooks.
Here is the output:
# GIT_TRACE=1 git cherry-pick 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
trace: built-in: git 'cherry-pick' '3f78d1f210ff89af77f042ab7f4a8fee39feb1c9'
trace: run_command: 'commit' '-n' '-F' '.git/MERGE_MSG'
trace: exec: 'git' 'commit' '-n' '-F' '.git/MERGE_MSG'
setup: git_dir: .git
setup: worktree: /home/dichtel/DEV/linux-2.6
setup: cwd: /home/dichtel/DEV/linux-2.6
setup: prefix: (null)
trace: built-in: git 'commit' '-n' '-F' '.git/MERGE_MSG'
[master 8372873] drivers/net/usb/asix.c: Fix unaligned accesses
1 files changed, 33 insertions(+), 1 deletions(-)
#
>
> I can't think of anything config-wise that would cause this behavior.
With a very basic config, the pb he still here:
cat ~/.gitconfig
[user]
name = Nicolas Dichtel
email = nicolas.dichtel@6wind.com
I will try to do some other tests.
Regards,
Nicolas
^ permalink raw reply
* [RFC/PATCH] remote-curl: Obey passed URL
From: Michael J Gruber @ 2011-10-06 13:15 UTC (permalink / raw)
To: git; +Cc: Shawn O. Pearce, Tay Ray Chuan, Jeff King
In-Reply-To: <4E8D4BD5.2090202@drmicha.warpmail.net>
When the curl remote helper is called, e.g., as
git-remote-https foo https://john@doe.com
it looks up remote.foo.url rather than taking the provided url, at least
as far as http_init() is concerned (authentication). (It does use the
provided url at later stages.)
The problem is that for pushing, "git push" looks up the pushurl, which
may be different, and passes that down to the remote helper, so that the
remote helper should take that when provided.
Note that at the init stage, the remote helper does not know what kind of
transcation is going to be requested, but the caller does.
Change it so that the remote helper obeys the passed url rather than
trying to look it up.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
While this passes all tests and fixes the described problem, I don't
know about any side effects. Also, I always feel a bit insecure about
simply dropping pointers to allocated memory (remote->url[0]). Should
I free() it? Neither remote-helpers nor memory management are exactly
homeground for me...
---
remote-curl.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index b8cf45a..1fa396a 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -846,6 +846,7 @@ int main(int argc, const char **argv)
if (argc > 2) {
end_url_with_slash(&buf, argv[2]);
+ remote->url[0] = xstrdup(argv[2]);
} else {
end_url_with_slash(&buf, remote->url[0]);
}
--
1.7.7.rc2.451.g15e150
^ permalink raw reply related
* Re: git-cherry-pick and git-commit --amend in version 1.7.6.4
From: Jay Soffian @ 2011-10-06 13:09 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <4E8D5AD0.2040509@6wind.com>
On Thu, Oct 6, 2011 at 3:37 AM, Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
> # ls .git
> branches COMMIT_EDITMSG config description FETCH_HEAD HEAD hooks
> index info logs objects ORIG_HEAD packed-refs refs
No CHERRY_PICK_HEAD, so far so good.
> # git cherry-pick 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
> [dev 4cca2c2] drivers/net/usb/asix.c: Fix unaligned accesses
> 1 files changed, 33 insertions(+), 1 deletions(-)
cherry-pick completes successfully.
> # ls .git
> branches CHERRY_PICK_HEAD COMMIT_EDITMSG config description FETCH_HEAD
> HEAD hooks index info logs objects ORIG_HEAD packed-refs refs
This is bad. CHERRY_PICK_HEAD should only exist if the cherry-pick failed.
I really don't know what could cause this. Possibly a hook in your repo?
Using "GIT_TRACE=1 git cherry-pick
3f78d1f210ff89af77f042ab7f4a8fee39feb1c9" will tell you whether git is
running any hooks.
I can't think of anything config-wise that would cause this behavior.
I'll peer at the code some more...
j.
^ permalink raw reply
* Re: [PATCH v3 3/4] enter_repo: do not modify input
From: Erik Faye-Lund @ 2011-10-06 13:06 UTC (permalink / raw)
To: Phil Hord; +Cc: git, peff, j6t, gitster, rene.scharfe
In-Reply-To: <CABURp0qDsxHwsuyvB6-KvKPrKuUT0-Fpr730TD_TxxFY7fotpA@mail.gmail.com>
On Tue, Oct 4, 2011 at 7:55 PM, Phil Hord <phil.hord@gmail.com> wrote:
> On Thu, Sep 29, 2011 at 4:59 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>> entr_repo(..., 0) currently modifies the input to strip away
>> trailing slashes. This means that we some times need to copy the
>> input to keep the original.
>
> I'm also modifying enter_repo() so I'm looking a bit closer at this patch now.
>
>> Change it to unconditionally copy it into the used_path buffer so
>> we can safely use the input without having to copy it.
>>
>> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
>> ---
> [...]
>> */
>> -char *enter_repo(char *path, int strict)
>> +const char *enter_repo(const char *path, int strict)
>> {
>> static char used_path[PATH_MAX];
>> static char validated_path[PATH_MAX];
>> @@ -297,14 +297,15 @@ char *enter_repo(char *path, int strict)
>> };
>> int len = strlen(path);
>> int i;
>> - while ((1 < len) && (path[len-1] == '/')) {
>> - path[len-1] = 0;
>> + while ((1 < len) && (path[len-1] == '/'))
>> len--;
>> - }
>> +
>> if (PATH_MAX <= len)
>> return NULL;
>> - if (path[0] == '~') {
>> - char *newpath = expand_user_path(path);
>> + strncpy(used_path, path, len);
>
> When len < strlen(path), this will will leave used_path unterminated.
>
Good catch, thanks!
>> +
>> + if (used_path[0] == '~') {
>> + char *newpath = expand_user_path(used_path);
>> if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
>> free(newpath);
>> return NULL;
>> @@ -316,24 +317,21 @@ char *enter_repo(char *path, int strict)
>> * anyway.
>> */
>> strcpy(used_path, newpath); free(newpath);
>> - strcpy(validated_path, path);
>> - path = used_path;
>> + strcpy(validated_path, used_path);
>
> The point of 'validated_path' is to keep the original unmolested,
> unexpanded path string (plus DWIM suffix), but here you've just
> replaced validated_path with a copy of the expanded_user_path. On the
> other hand, we seem always to strcpy(validated_path , path), so we
> might as well get that done up-front.
Yeah, that's probably better.
^ permalink raw reply
* [PATCH] git-difftool: allow skipping file by typing 'n' at prompt
From: Sitaram Chamarty @ 2011-10-06 12:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Phil Hord, Sitaram Chamarty, git
In-Reply-To: <CAMK1S_gssgpy7nF46c1roJUCN5yvQaOYfVE_-ZrvMfHGWKvk0w@mail.gmail.com>
Signed-off-by: Sitaram Chamarty <sitaram@atc.tcs.com>
---
(re-rolled according to earlier discussion)
git-difftool--helper.sh | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
index 8452890..0468446 100755
--- a/git-difftool--helper.sh
+++ b/git-difftool--helper.sh
@@ -38,15 +38,16 @@ launch_merge_tool () {
# $LOCAL and $REMOTE are temporary files so prompt
# the user with the real $MERGED name before launching $merge_tool.
+ ans=y
if should_prompt
then
printf "\nViewing: '$MERGED'\n"
if use_ext_cmd
then
- printf "Hit return to launch '%s': " \
+ printf "Launch '%s' [Y/n]: " \
"$GIT_DIFFTOOL_EXTCMD"
else
- printf "Hit return to launch '%s': " "$merge_tool"
+ printf "Launch '%s' [Y/n]: " "$merge_tool"
fi
read ans
fi
@@ -54,9 +55,9 @@ launch_merge_tool () {
if use_ext_cmd
then
export BASE
- eval $GIT_DIFFTOOL_EXTCMD '"$LOCAL"' '"$REMOTE"'
+ test "$ans" != "n" && eval $GIT_DIFFTOOL_EXTCMD '"$LOCAL"' '"$REMOTE"'
else
- run_merge_tool "$merge_tool"
+ test "$ans" != "n" && run_merge_tool "$merge_tool"
fi
}
--
1.7.6
^ permalink raw reply related
* Re: git-cherry-pick and author field in version 1.7.6.4
From: Nicolas Dichtel @ 2011-10-06 12:37 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111006112742.GA4445@sigill.intra.peff.net>
Le 06/10/2011 13:27, Jeff King a écrit :
> On Thu, Oct 06, 2011 at 09:51:06AM +0200, Nicolas Dichtel wrote:
>
>> Here is my sequence. I'm in a linux tree with a remote that point to
>> linus tree and I want to cherry-pick a patch from this remote:
>>
>> # git cherry-pick 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
>> [dev 87ce387] drivers/net/usb/asix.c: Fix unaligned accesses
>> 1 files changed, 33 insertions(+), 1 deletions(-)
>> # git log -1 --format='%an<%ae>'
>> Nicolas Dichtel<nicolas.dichtel@6wind.com>
>> # git log -1 --format='%an<%ae>' 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
>> Neil Jones<NeilJay@gmail.com>
>> #
>
> Hmph. Odd:
>
> $ cd linux-2.6
> $ git checkout -b dev 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9^
> Switched to a new branch 'dev'
> $ git cherry-pick 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
> [dev 78929c2] drivers/net/usb/asix.c: Fix unaligned accesses
> Author: Neil Jones<NeilJay@gmail.com>
> 1 files changed, 33 insertions(+), 1 deletions(-)
> $ git log -1 --format='%an<%ae>'
> Neil Jones<NeilJay@gmail.com>
# git checkout -b dev 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9^
Checking out files: 100% (25721/25721), done.
Switched to a new branch 'dev'
# git cherry-pick 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
[dev 693df4c] drivers/net/usb/asix.c: Fix unaligned accesses
1 files changed, 33 insertions(+), 1 deletions(-)
# git log -1 --format='%an<%ae>'
Nicolas Dichtel<nicolas.dichtel@6wind.com>
#
>
>> Maybe it is related to the problem I've reported in another thread:
>> http://comments.gmane.org/gmane.comp.version-control.git/182853
>
> Possibly. That issue is about the commit that comes _after_ the
> cherry-pick, and in this instance, things are already wrong for you by
> the time the cherry-pick has completed.
>
> However, the problem has to do with leaving a stale state file in .git,
> so perhaps a previous partially-completed cherry-pick has left cruft in
> .git that is confusing this cherry-pick (i.e., I can't reproduce because
> it is being affected by something that happened before the commands
> above). So let's see what Jay comes up with for solving the other
> problem, and I suspect it may just fix this issue, too.
I think so too. Will wait.
>
> You might also repeating the commands above. If it still fails, maybe
> try removing ".git/CHERRY_PICK_HEAD" if it exists and see if that helps.
No, it just allow the commit --amend, but this will not change the author.
Regards,
Nicolas
^ permalink raw reply
* Re: git-cherry-pick and author field in version 1.7.6.4
From: Jeff King @ 2011-10-06 11:27 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: git
In-Reply-To: <4E8D5DEA.9010500@6wind.com>
On Thu, Oct 06, 2011 at 09:51:06AM +0200, Nicolas Dichtel wrote:
> Here is my sequence. I'm in a linux tree with a remote that point to
> linus tree and I want to cherry-pick a patch from this remote:
>
> # git cherry-pick 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
> [dev 87ce387] drivers/net/usb/asix.c: Fix unaligned accesses
> 1 files changed, 33 insertions(+), 1 deletions(-)
> # git log -1 --format='%an<%ae>'
> Nicolas Dichtel<nicolas.dichtel@6wind.com>
> # git log -1 --format='%an<%ae>' 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
> Neil Jones<NeilJay@gmail.com>
> #
Hmph. Odd:
$ cd linux-2.6
$ git checkout -b dev 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9^
Switched to a new branch 'dev'
$ git cherry-pick 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
[dev 78929c2] drivers/net/usb/asix.c: Fix unaligned accesses
Author: Neil Jones <NeilJay@gmail.com>
1 files changed, 33 insertions(+), 1 deletions(-)
$ git log -1 --format='%an <%ae>'
Neil Jones <NeilJay@gmail.com>
> Maybe it is related to the problem I've reported in another thread:
> http://comments.gmane.org/gmane.comp.version-control.git/182853
Possibly. That issue is about the commit that comes _after_ the
cherry-pick, and in this instance, things are already wrong for you by
the time the cherry-pick has completed.
However, the problem has to do with leaving a stale state file in .git,
so perhaps a previous partially-completed cherry-pick has left cruft in
.git that is confusing this cherry-pick (i.e., I can't reproduce because
it is being affected by something that happened before the commands
above). So let's see what Jay comes up with for solving the other
problem, and I suspect it may just fix this issue, too.
You might also repeating the commands above. If it still fails, maybe
try removing ".git/CHERRY_PICK_HEAD" if it exists and see if that helps.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] Add multiple workdir support to branch/checkout
From: Bernhard R. Link @ 2011-10-06 11:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jay Soffian, Nguyen Thai Ngoc Duy, git
In-Reply-To: <7vzkhf713u.fsf@alter.siamese.dyndns.org>
* Junio C Hamano <gitster@pobox.com> [111005 20:19]:
> I do not necessarily think that it is a good approach to forbid the same
> branch to be checked out in two different places, by the way. [...]
> [...] The problem arises only when one of the repositories try
> to update or delete the branch while it is checked out in another working
> tree.
I think this is mostly the same problem that also make pushing to a
checked out branch a problem.
Isn't the real problem that a checked out branch / a branch having a
workdir only has information what branch it belongs to?
Wouldn't both problems (multiple workdirs of the same branch, pushing
to a checked out branch) solved if each working directory (including
the default one in a non-bare repository) also stored the commit id
last checked out? (And then giving a warning, error or automatically
creating a detached head setting whenever the branch it followed is
moved behind it's back?)
Bernhard R. Link
^ permalink raw reply
* Re: [BUG] git stash -k show the help message for diff-index
From: Matthieu Moy @ 2011-10-06 9:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4nzn8hcp.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Hmm, does not reproduce.
>
> : alter victim-2.git/master; git status
> # On branch master
> nothing to commit (working directory clean)
> : alter victim-2.git/master; git stash -k
> No local changes to save
> : alter victim-2.git/master; git version
> git version 1.7.7
Sorry, I should have mentionned that: I had the bug with next.
I've tried bisecting it down, but the bug mysteriously disappeared
during the bisect (I did have bad and good commits during bisect, but at
the end, I had only good commits, and the bad ones weren't
reproducible).
I can't reproduce anymore.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH 0/9] i18n: add PO files to po/
From: Peter Krefting @ 2011-10-06 9:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: Ævar Arnfjörð Bjarmason, Jonathan Nieder,
Git Mailing List, Ramkumar Ramachandra, Marcin Cieślak,
Sam Reed, Jan Engelhardt, Jan Krüger,
Nguyễn Thái Ngọc
In-Reply-To: <7vaa9gbdyc.fsf@alter.siamese.dyndns.org>
Junio C Hamano:
> These two are somewhat mutually exclusive. I _suspect_ that Jonathan might
> have been hinting me to eject everything under the current po/ directory,
> and bind that part of the tree as a submodule from another repository,
> which would give us "git log -p" cleanliness automatically.
That sounds like a good idea, actually. That way you can also chose which
version of the po/ tree to include without having to do crazy cherry-picking
and stuff, and it makes it easier to maintain externally and such.
I have experience working with Translation Project, both as a software
maintainer requesting translation, and as a translator doing translations,
and I am interested in setting up a po repository for Git and use TP to
maintain translations (or directly, for those that would prefer that).
--
\\// Peter - volunteered, because localization is a good thing
^ permalink raw reply
* git-svn: ignore-paths not enough (not ignoring refs?)
From: Piotr Krukowiecki @ 2011-10-06 8:43 UTC (permalink / raw)
To: Michael Olson; +Cc: Eric Wong, Git Mailing List
Hi,
I'm using --ignore-paths options but some paths which should be
ignored (they math the regexp) are not ignored. I suspect this is
because of http://thread.gmane.org/gmane.comp.version-control.git/145398
It seems the patch from that url was never accepted to git. It does
not apply anymore too. Is it possible to update the patch? From the
discussion the patch looked ok (with exception to git-svn dying if a
parent ref didn't exist :) )
Here's my git-svn clone log, with some print lines added to is_path_ignored():
$ git svn clone -s --no-follow-parent --ignore-paths='$REGEXP -r 1230:1240 $URL
[...]
is_path_ignored: 'xtest/tags/rel1' # this matches the regexp and
should be ignored
is_path_ignored: 'xtest/tags/rel1/common' # more paths which should be
ignored follow
[...]
r1237 = c660a7e6ad81cafa0a64a7ec85a3e23f0c02bc09
(refs/remotes/tags/rel1) # but it is not!
# I can see the tag:
$ git branch -a
remotes/tags/rel1
# There are no changes in this change:
$ git show -p c660a7e6ad81cafa0a64a7ec85a3e23f0c02bc09
[...]
git-svn-id: $URL/rel1@1237 3e523551-a86b-4873-bcb6-76fcd93a4c5c
# Under svn the r1237 is following:
$ svn log -r 1237 -v $URL
------------------------------------------------------------------------
r1237 | ...
Changed paths:
A /xtest/tags/rel1/common (from /xtest/branches/xtest44/common:1235)
# The xtest/branches/xtest44/common does not match the ignore-paths
and is not ignored.
--
Piotr Krukowiecki
^ permalink raw reply
* Re: git-cherry-pick and git-commit --amend in version 1.7.6.4
From: Nicolas Dichtel @ 2011-10-06 7:53 UTC (permalink / raw)
To: Jay Soffian, git
In-Reply-To: <4E8D5AD0.2040509@6wind.com>
Le 06/10/2011 09:37, Nicolas Dichtel a écrit :
> Le 05/10/2011 18:50, Jay Soffian a écrit :
>> On Wed, Oct 5, 2011 at 10:52 AM, Nicolas Dichtel
>> <nicolas.dichtel@6wind.com> wrote:
>>> Hi,
>>>
>>> still with version 1.7.6.4, when I do a cherry-pick, that succeeded, I
>>> cannot do a commit --amend after:
>>>
>>> # git cherry-pick 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
>>> [dev 1a04a23] drivers/net/usb/asix.c: Fix unaligned accesses
>>> 1 files changed, 33 insertions(+), 1 deletions(-)
>>> # echo $?
>>> 0
>>> # git commit --amend
>>> fatal: You are in the middle of a cherry-pick -- cannot amend.
>>> #
>>>
>>> The same operations (with the same patch), with version 1.7.3.4 is ok.
>>
>> Please do the following with 1.7.6.4:
>>
>> # ls .git
>> # git cherry-pick 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
>> # ls .git
>> # git cat-file -p 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
>> # git cat-file -p HEAD
>>
>> And send the transcript.
> Here is:
>
> # ls .git
> branches COMMIT_EDITMSG config description FETCH_HEAD HEAD hooks index info logs
> objects ORIG_HEAD packed-refs refs
> # git cherry-pick 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
> [dev 4cca2c2] drivers/net/usb/asix.c: Fix unaligned accesses
> 1 files changed, 33 insertions(+), 1 deletions(-)
> # ls .git
> branches CHERRY_PICK_HEAD COMMIT_EDITMSG config description FETCH_HEAD HEAD
> hooks index info logs objects ORIG_HEAD packed-refs refs
> # git cat-file -p 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
> tree f29742a1a73c27a88c7ac701a7a06ac1c2f7973a
> parent e7a3af5d8cd782b84e6ca4e4dcc8613be1a809f0
> author Neil Jones <NeilJay@gmail.com> 1274141908 -0700
> committer David S. Miller <davem@davemloft.net> 1274141908 -0700
>
> drivers/net/usb/asix.c: Fix unaligned accesses
>
> Using this driver can cause unaligned accesses in the IP layer
> This has been fixed by aligning the skb data correctly using the
> spare room left over by the 4 byte header inserted between packets
> by the device.
>
> Signed-off-by: Neil Jones <NeilJay@gmail.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> # git cat-file -p HEAD
> tree 282b6492d9d5bcf1c3718420c6f31ca2033ca5cb
> parent c8054f854773e65d8592f2ef35939ec2ae8b01df
> author Nicolas Dichtel <nicolas.dichtel@6wind.com> 1317886553 +0200
> committer Nicolas Dichtel <nicolas.dichtel@6wind.com> 1317886553 +0200
>
> drivers/net/usb/asix.c: Fix unaligned accesses
>
> Using this driver can cause unaligned accesses in the IP layer
> This has been fixed by aligning the skb data correctly using the
> spare room left over by the 4 byte header inserted between packets
> by the device.
>
> Signed-off-by: Neil Jones <NeilJay@gmail.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> #
>
There is another symptom, describe in this thread:
http://comments.gmane.org/gmane.comp.version-control.git/182852
Maybe the two problems are related.
Regards,
Nicolas
^ permalink raw reply
* Re: git-cherry-pick and author field in version 1.7.6.4
From: Nicolas Dichtel @ 2011-10-06 7:51 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111005174138.GA22962@sigill.intra.peff.net>
Le 05/10/2011 19:41, Jeff King a écrit :
> On Wed, Oct 05, 2011 at 04:51:58PM +0200, Nicolas Dichtel wrote:
>
>> in the last stable version (1.7.6.4), when I perform a
>> git-cherry-pick, the initial author of the patch is erased whith my
>> name (it was not the case in version 1.7.3.4 and prior). Is this
>> behavior intended ? Is there an option to keep the initial author of
>> the patch?
>
> I can't reproduce your problem:
>
> git init repo&&
> cd repo&&
> echo content>file&& git add file&& git commit -m base&&
> echo changes>>file&&
> git commit --author='Other Person<other@example.com>' -a -m other&&
> git tag other&&
> git reset --hard HEAD^&&
> git cherry-pick other
>
> gives this output for the cherry-pick:
>
> [master 6eb207f] other
> Author: Other Person<other@example.com>
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> and the resulting commit looks good:
>
> $ git log -1 --format='%an<%ae>'
> Other Person<other@example.com>
>
> Does the script above work for you? If so, then what is different about
> your problematic case?
Here is my sequence. I'm in a linux tree with a remote that point to linus tree
and I want to cherry-pick a patch from this remote:
# git cherry-pick 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
[dev 87ce387] drivers/net/usb/asix.c: Fix unaligned accesses
1 files changed, 33 insertions(+), 1 deletions(-)
# git log -1 --format='%an<%ae>'
Nicolas Dichtel<nicolas.dichtel@6wind.com>
# git log -1 --format='%an<%ae>' 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
Neil Jones<NeilJay@gmail.com>
#
Maybe it is related to the problem I've reported in another thread:
http://comments.gmane.org/gmane.comp.version-control.git/182853
Regards,
Nicolas
^ permalink raw reply
* Re: git-cherry-pick and git-commit --amend in version 1.7.6.4
From: Nicolas Dichtel @ 2011-10-06 7:37 UTC (permalink / raw)
To: Jay Soffian; +Cc: git
In-Reply-To: <CAG+J_DynqAK8uXDPtHwWpGhfA5qFZifucs91qL79Pu_DmCxG3g@mail.gmail.com>
Le 05/10/2011 18:50, Jay Soffian a écrit :
> On Wed, Oct 5, 2011 at 10:52 AM, Nicolas Dichtel
> <nicolas.dichtel@6wind.com> wrote:
>> Hi,
>>
>> still with version 1.7.6.4, when I do a cherry-pick, that succeeded, I
>> cannot do a commit --amend after:
>>
>> # git cherry-pick 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
>> [dev 1a04a23] drivers/net/usb/asix.c: Fix unaligned accesses
>> 1 files changed, 33 insertions(+), 1 deletions(-)
>> # echo $?
>> 0
>> # git commit --amend
>> fatal: You are in the middle of a cherry-pick -- cannot amend.
>> #
>>
>> The same operations (with the same patch), with version 1.7.3.4 is ok.
>
> Please do the following with 1.7.6.4:
>
> # ls .git
> # git cherry-pick 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
> # ls .git
> # git cat-file -p 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
> # git cat-file -p HEAD
>
> And send the transcript.
Here is:
# ls .git
branches COMMIT_EDITMSG config description FETCH_HEAD HEAD hooks index
info logs objects ORIG_HEAD packed-refs refs
# git cherry-pick 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
[dev 4cca2c2] drivers/net/usb/asix.c: Fix unaligned accesses
1 files changed, 33 insertions(+), 1 deletions(-)
# ls .git
branches CHERRY_PICK_HEAD COMMIT_EDITMSG config description FETCH_HEAD
HEAD hooks index info logs objects ORIG_HEAD packed-refs refs
# git cat-file -p 3f78d1f210ff89af77f042ab7f4a8fee39feb1c9
tree f29742a1a73c27a88c7ac701a7a06ac1c2f7973a
parent e7a3af5d8cd782b84e6ca4e4dcc8613be1a809f0
author Neil Jones <NeilJay@gmail.com> 1274141908 -0700
committer David S. Miller <davem@davemloft.net> 1274141908 -0700
drivers/net/usb/asix.c: Fix unaligned accesses
Using this driver can cause unaligned accesses in the IP layer
This has been fixed by aligning the skb data correctly using the
spare room left over by the 4 byte header inserted between packets
by the device.
Signed-off-by: Neil Jones <NeilJay@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
# git cat-file -p HEAD
tree 282b6492d9d5bcf1c3718420c6f31ca2033ca5cb
parent c8054f854773e65d8592f2ef35939ec2ae8b01df
author Nicolas Dichtel <nicolas.dichtel@6wind.com> 1317886553 +0200
committer Nicolas Dichtel <nicolas.dichtel@6wind.com> 1317886553 +0200
drivers/net/usb/asix.c: Fix unaligned accesses
Using this driver can cause unaligned accesses in the IP layer
This has been fixed by aligning the skb data correctly using the
spare room left over by the 4 byte header inserted between packets
by the device.
Signed-off-by: Neil Jones <NeilJay@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
#
Regards,
Nicolas
^ permalink raw reply
* Re: git-cherry-pick and git-commit --amend in version 1.7.6.4
From: Junio C Hamano @ 2011-10-06 7:06 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, nicolas.dichtel
In-Reply-To: <CAG+J_Dysix9fOCuvm5+aU7-AC4wmsxH4-MOX+yhaHEqzeN1cPg@mail.gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
> Something like this?
>
> diff --git i/builtin/revert.c w/builtin/revert.c
> index 3117776c2c..f7fcc88871 100644
> --- i/builtin/revert.c
> +++ w/builtin/revert.c
> @@ -384,6 +384,7 @@ static int do_pick_commit(void)
> char *defmsg = NULL;
> struct strbuf msgbuf = STRBUF_INIT;
> int res;
> + int record_cherry_pick_head = 0;
>
> if (no_commit) {
> /*
> @@ -477,7 +478,7 @@ static int do_pick_commit(void)
> strbuf_addstr(&msgbuf, ")\n");
> }
> if (!no_commit)
> - write_cherry_pick_head();
> + record_cherry_pick_head = 1;
> }
>
> if (!strategy || !strcmp(strategy, "recursive") || action == REVERT) {
> @@ -514,6 +515,9 @@ static int do_pick_commit(void)
> free_message(&msg);
> free(defmsg);
>
> + if (record_cherry_pick_head)
> + write_cherry_pick_head();
> +
> return res;
> }
I switched to "maint" to look at this patch in context without the
sequencer complication.
The basic idea to delay writing the file feels sound, but when a conflict
happens, print_advice() runs and tries to clear CHERRY_PICK_HEAD, but you
are then writing the file out much later than that at the end of the
function.
This patch seems to break a few tests. t3404, t3506 and t3507 are among
them.
Also, if you are using the recursive strategy, a cherry-pick that did not
start would die() in do_recursive_merge(), and your hunk at -477,7 to
remove call to write_cherry_head() would be sufficient, but if you are
using another strategy, then try_merge_command() would return with 2 and I
think you would want to skip it for the same reason in that case.
^ permalink raw reply
* Re: [PATCH 2/4] cleanup: use internal memory allocation wrapper functions everywhere
From: Alexey Shumkin @ 2011-10-06 7:01 UTC (permalink / raw)
To: Johannes Sixt
Cc: Brandon Casey, peff@peff.net, git@vger.kernel.org,
gitster@pobox.com, sunshine@sunshineco.com, bharrosh@panasas.com,
trast@student.ethz.ch
In-Reply-To: <4E8D4812.9090102@viscovery.net>
Offtopic:
Excuse me, guys
May I ask you to exclude me from CC-list for this topic,
it seems to me I got in it for a mistake somehow,
I have no relation to this topic
thanks in advance :)
> Am 10/6/2011 4:00, schrieb Brandon Casey:
> > [resend without html bits added by "gmail offline"]
> > On Wed, Oct 5, 2011 at 7:53 PM, Brandon Casey <drafnel@gmail.com>
> > wrote:
> >> On Thursday, September 15, 2011, Brandon Casey wrote:
> >>>
> >>> On Thu, Sep 15, 2011 at 1:52 AM, Johannes Sixt
> >>> <j.sixt@viscovery.net>
> >>>> There is a danger that the high-level die() routine (which is
> >>>> used by the
> >>>> x-wrappers) uses one of the low-level compat/ routines. IOW, in
> >>>> the case of errors, recursion might occur. Therefore, I would
> >>>> prefer that the compat/ routines do their own error reporting
> >>>> (preferably via return values and errno).
> >>>
> >>> Thanks. Will do.
> >>
> >> Hi Johannes,
> >> I have taken a closer look at the possibility of recursion with
> >> respect to die() and the functions in compat/. It appears the
> >> risk is only related to vsnprintf/snprintf at the moment. So as
> >> long as we avoid calling xmalloc et al from within snprintf.c, I
> >> think we should be safe from recursion. I'm inclined to keep the
> >> additions to mingw.c and win32/syslog.c since they both already
> >> use the x-wrappers or strbuf, even though they could easily be
> >> worked around. The other file that was touched is compat/qsort,
> >> which returns void and doesn't have a good alternative error
> >> handling path. So, I'm inclined to keep that one too.
>
> I'm fine with keeping the change to mingw.c (getaddrinfo related) and
> qsort: both are unlikely to be called from die().
>
> But syslog() *is* called from die() in git-daemon, and it would be
> better to back out the other offenders instead of adding to them.
>
> -- Hannes
>
^ permalink raw reply
* Re: Git ksshaskpass to play nice with https and kwallet
From: Michael J Gruber @ 2011-10-06 6:33 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20111005180125.GC22962@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 05.10.2011 20:01:
> On Wed, Oct 05, 2011 at 01:55:36PM -0400, Jeff King wrote:
>
>> On Tue, Oct 04, 2011 at 08:49:55PM +0200, Michael J Gruber wrote:
>>
>>> We seem to mean something different:
>>>
>>> git config --get remote.bitbucket.pushurl
>>> https://grubix@bitbucket.org/grubix/git.git
>>> SSH_ASKPASS= git push -n bitbucket
>>> Username for 'bitbucket.org':
>>>
>>> I mean that git should not need to ask for the username here.
>>
>> No, we are in agreement about the intended behavior. I think you are
>> seeing a bug. What version of git produced it?
>>
>> With my http-auth series, I get:
>>
>> $ git push https://github.com/peff/git.git
>> Username for 'github.com':
>>
>> $ git push https://peff@github.com/peff/git.git
>> Password for 'github.com':
>>
>> Using v1.7.7 produces similar results.
>
> Hrm. I do get this, with the same version of git:
>
> $ git config remote.foo.url https://github.com/peff/git.git
> $ git push foo
> Username for 'github.com':
>
> $ git config remote.foo.url https://peff@github.com/peff/git.git
> $ git push foo
> Password for 'github.com':
>
> So far so good. Now how about this:
>
> $ git config remote.foo.url https://github.com/peff/git.git
> $ git config remote.foo.pushurl https://peff@github.com/peff/git.git
> $ git push foo
> Username for 'github.com':
>
> So I think the problem is with pushurl, not with the auth code. Oddly,
> though, running GIT_TRACE reveals:
Yep, I have a pushurl in config.
>
> $ GIT_TRACE=1 git push foo
> trace: built-in: git 'push' 'foo'
> trace: run_command: 'git-remote-https' 'foo' 'https://peff@github.com/peff/git.git'
>
> which is the right URL. So it's almost as if we are throwing away the
> passed URL in favor of the configuration, and then looking up the
> configuration wrong. I'm about to go get on a plane, so I don't have
> more time to look at it now, but I suspect it's something simple and
> stupid.
Thanks for confirming what it should be like. I never expected pushurl
and url to behave differently (I had used GIT_TRACE myself).
Good flight :)
Michael
^ permalink raw reply
* Re: [PATCH 2/4] cleanup: use internal memory allocation wrapper functions everywhere
From: Johannes Sixt @ 2011-10-06 6:17 UTC (permalink / raw)
To: Brandon Casey
Cc: peff@peff.net, git@vger.kernel.org, gitster@pobox.com,
sunshine@sunshineco.com, bharrosh@panasas.com,
trast@student.ethz.ch, zapped@mail.ru
In-Reply-To: <CA+sFfMdHpvdMU==a2sUR9sZZCcgqPfGF7+dy6yi8RVoMZ+uZVA@mail.gmail.com>
Am 10/6/2011 4:00, schrieb Brandon Casey:
> [resend without html bits added by "gmail offline"]
> On Wed, Oct 5, 2011 at 7:53 PM, Brandon Casey <drafnel@gmail.com> wrote:
>> On Thursday, September 15, 2011, Brandon Casey wrote:
>>>
>>> On Thu, Sep 15, 2011 at 1:52 AM, Johannes Sixt <j.sixt@viscovery.net>
>>>> There is a danger that the high-level die() routine (which is used by
>>>> the
>>>> x-wrappers) uses one of the low-level compat/ routines. IOW, in the case
>>>> of errors, recursion might occur. Therefore, I would prefer that the
>>>> compat/ routines do their own error reporting (preferably via return
>>>> values and errno).
>>>
>>> Thanks. Will do.
>>
>> Hi Johannes,
>> I have taken a closer look at the possibility of recursion with respect to
>> die() and the functions in compat/. It appears the risk is only related to
>> vsnprintf/snprintf at the moment. So as long as we avoid calling xmalloc et
>> al from within snprintf.c, I think we should be safe from recursion.
>> I'm inclined to keep the additions to mingw.c and win32/syslog.c since they
>> both already use the x-wrappers or strbuf, even though they could easily be
>> worked around. The other file that was touched is compat/qsort, which
>> returns void and doesn't have a good alternative error handling path. So,
>> I'm inclined to keep that one too.
I'm fine with keeping the change to mingw.c (getaddrinfo related) and
qsort: both are unlikely to be called from die().
But syslog() *is* called from die() in git-daemon, and it would be better
to back out the other offenders instead of adding to them.
-- Hannes
^ permalink raw reply
* [PATCH] strbuf.c: remove unnecessary strbuf_grow() from strbuf_getwholeline()
From: Brandon Casey @ 2011-10-06 4:21 UTC (permalink / raw)
To: git; +Cc: Brandon Casey
This use of strbuf_grow() is a historical artifact that was once used to
ensure that strbuf.buf was allocated and properly nul-terminated. This
was added before the introduction of the slopbuf in b315c5c0, which
guarantees that strbuf.buf always points to a usable nul-terminated string.
So let's remove it.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
strbuf.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index 9ff1b59..3ad2cc0 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -357,7 +357,6 @@ int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
{
int ch;
- strbuf_grow(sb, 0);
if (feof(fp))
return EOF;
--
1.7.7.1.ge3b6f
^ permalink raw reply related
* Re: [RFC/PATCH] Add multiple workdir support to branch/checkout
From: Jay Soffian @ 2011-10-06 4:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <7v1uuq51c3.fsf@alter.siamese.dyndns.org>
On Wed, Oct 5, 2011 at 9:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jay Soffian <jaysoffian@gmail.com> writes:
>> Now they do what? Either commit --force or create a new branch?
>> Wouldn't it have been better to create the new branch before they
>> started editing?
>
> If they are going to commit, and if they knew that they are going to
> commit, yes.
Committing is obviously the common case for a checked-out branch.
> But why do you want to forbid people from just checking things out if they
> are not interested in committing? That is where I think you are going
> backwards.
Because if they do decide to commit, it's now harder for them to do so.
It would be great if git could intervene after the checkout, but
before they edit any files, so that they don't have uncommitted work.
Obviously that's not possible, so git should prevent them from getting
to that point.
Let's consider the various situations:
1. master is checked out w/edits in workdir1, user wants to work on
topic in workdir2.
There's nothing to warn about in workdir2 neither at checkout nor commit time.
2. master is checked out w/edits in workdir1, user wants examine
unedited master in workdir2
At checkout time in workdir2:
My preference: checkout advices user to use --detach or --force.
Your preference: checkout is silent.
Now user decides they want to commit to master in workdir2 (which is
insane, they've got uncommitted changes to it in workdir1). What
happens?
In my scenario, the commit happens on a detached HEAD. When they
eventually switch back to a branch, git tells them how to move their
commit to a branch.
In your scenario, commit complains. User now has to --force, stash, or
create a new branch.
It's just seems insane to me putting in obstacles to the user
committing their work. That's where I think you are going backwards.
You have a use case where using a detached HEAD doesn't work because
you've scripted around the same branch multiply checked out. I think
that's probably an exceedingly rare use case, and justifies "checkout
--force".
>> I guess it depends what you mostly use your workdirs for. For me, it's
>> to have different branches checked out, not to have the same branch
>> checked out in multiple locations.
>
> Then you wouldn't have any problem if commit refused to make commit on the
> branch that is checked out elsewhere, no?
Yes, I would, because by that point, I've already made the mistake of
checking out the same branch twice. I want git to prevent me from
doing that by accident. Because I don't want to ever be in the
situation which comes next, which is that I've got uncommitted work
for the same branch in two places!
> I am not saying we should never have an option to _warn_ checking out the
> same branch in multiple places. I am saying it is wrong to forbid doing so
> by default.
I am not saying we should never have an option to allow checking out
the same branch in multiple places. I am saying it is wrong to allow
doing so by default.
j.
^ 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