* Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions
From: Uwe Kleine-König @ 2009-03-12 17:04 UTC (permalink / raw)
To: Brian Campbell; +Cc: Junio C Hamano, git, Petr Baudis
In-Reply-To: <F3665462-16BC-42D6-BE28-F66CF48CEB9B@dartmouth.edu>
Hello Brian,
On Thu, Mar 12, 2009 at 12:41:45PM -0400, Brian Campbell wrote:
>> IMHO we should reuse as much as possible from git.git. For me even
>> requiring a git.git checkout to use its files would be OK. I consider
>> that even better then duplicating the relevant files.
>
> Hmm. How would the tests find your git working tree? I'd be willing to
> start the process off at least by writing test cases for the
> functionality I'm changing here if I had a good idea of how to start.
> Would it be sufficient to make something like "GIT_CHECKOUT=~/src/git
> make check" work?
Yes, this would be a good start. I would call it GIT_SRC, but that's up
to you.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: git checkout -b origin/mybranch origin/mybranch
From: John Tapsell @ 2009-03-12 16:58 UTC (permalink / raw)
To: Jeff King; +Cc: Pieter de Bie, Sverre Rabbelier, Johannes Schindelin, Git List
In-Reply-To: <20090312165153.GA28401@coredump.intra.peff.net>
2009/3/12 Jeff King <peff@peff.net>:
> On Thu, Mar 12, 2009 at 04:40:10PM +0000, Pieter de Bie wrote:
>
>>> I think the future-proofing is probably not worth the effort.
>>> Catching things that are ambiguous _now_ will cover the "oops, I
>>> typed the wrong thing" case, which I think is really the issue.
>>
>> Exactly, that's the common case where things go wrong. I guess using
>> dwim_ref should be DWIM enough? :)
>
> Hmm. Yeah, I mispoke before: I should have said dwim_ref instead of
> resolve_ref (which doesn't dwym :) ).
>
> Here's a sloppy patch that I think does what you want; but it might make
> more sense to just iterate over ref_rev_parse_rules ourselves, as
> dwim_ref does more than we care about (and we should probably
> differentiate between "a branch already exists" and "this would make an
> ambiguous ref").
>
> ---
> diff --git a/branch.c b/branch.c
> index d20fb04..409f445 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -122,6 +122,7 @@ void create_branch(const char *head,
> unsigned char sha1[20];
> char *real_ref, msg[PATH_MAX + 20];
> struct strbuf ref = STRBUF_INIT;
> + char *junk;
> int forcing = 0;
> int len;
>
> @@ -135,7 +136,8 @@ void create_branch(const char *head,
> if (check_ref_format(ref.buf))
> die("'%s' is not a valid branch name.", name);
>
> - if (resolve_ref(ref.buf, sha1, 1, NULL)) {
> + if (dwim_ref(name, strlen(name), sha1, &junk)) {
> + free(junk);
Presumably 'junk' is the resolved name? I wonder if it's worth
putting this info in the error message?
> if (!force)
> die("A branch named '%s' already exists.", name);
> die("A branch named '%s' already exists (%s).", name, junk);
That would give "A branched named 'origin/master' already exists
(refs/remotes/origin/master)" right?
Dunno if it's worth it, just wondering.
^ permalink raw reply
* Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions
From: Brian Campbell @ 2009-03-12 16:41 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: Junio C Hamano, git, Petr Baudis
In-Reply-To: <20090312152039.GA15626@pengutronix.de>
On Mar 12, 2009, at 11:20 AM, Uwe Kleine-König wrote:
>>>> To fix these, you might want to do something like:
>>>>
>>>> if head_=$(git symbolic-ref HEAD)
> Shouldn't git symbolic-ref -q HEAD be used here?
Yes, most likely.
>>>> then
>>>> case "$head_" in
>>>> refs/heads/*)
>>>> echo "${head_#refs/heads/}"
>>>> ;;
>>>> refs/top-bases/*)
>>>> echo "${head_#refs/top-bases/}"
>>>> ;;
>>>> *)
>>>> echo "$head_"
>>>> ;;
>>>> esac
>>>> else
>>>> whatever you want to do on a detached HEAD
> How do I distinguish between a detached HEAD and another error? I
> have
> the feeling that git symbolic-ref -q HEAD should exit(0) with a
> detached
> HEAD.
If you pass -q, it exits with status 1 on a detached head, 128 on
other errors, so we can use that to distinguish.
>
>>> Thanks Junio and Brian.
>>>
>>> Brian, do you update the series?
>>
>> Sure, I'll send an updated patch.
>>
>> I'm thinking that for the detached HEAD case, this function should
>> die
>> with a message about not being on a valid branch, and then the call
>> site
>> in tg-summary that doesn't care about being on a valid branch should
>> ignore the error and leave curname empty. Does that sound about
>> right?
> mmh, I would return "" and let the caller handle that.
Fair enough.
>> Also, has anyone considered writing a test suite for TopGit?
> Yes, but I didn't found the time for that until now. If you'd
> volunteer
> that would be very welcome.
I would, but I'm not sure I'll be continuing to use TopGit for more
than the one patch series I'm using it for now; I was trying it out,
but it feels a little more heavy-weight than what I want. StGIT or
just rewriting a patch series with git rebase -i works better for my
uses; I'm not maintaining a lot of long-lived topic branches upon
which I need full history.
> IMHO we should reuse as much as possible from git.git. For me even
> requiring a git.git checkout to use its files would be OK. I consider
> that even better then duplicating the relevant files.
Hmm. How would the tests find your git working tree? I'd be willing to
start the process off at least by writing test cases for the
functionality I'm changing here if I had a good idea of how to start.
Would it be sufficient to make something like "GIT_CHECKOUT=~/src/git
make check" work?
-- Brian
^ permalink raw reply
* Re: git checkout -b origin/mybranch origin/mybranch
From: Jeff King @ 2009-03-12 16:51 UTC (permalink / raw)
To: Pieter de Bie
Cc: John Tapsell, Sverre Rabbelier, Johannes Schindelin, Git List
In-Reply-To: <EAF5D3F1-32F2-4BF3-9B10-F91C17D06A6A@ai.rug.nl>
On Thu, Mar 12, 2009 at 04:40:10PM +0000, Pieter de Bie wrote:
>> I think the future-proofing is probably not worth the effort.
>> Catching things that are ambiguous _now_ will cover the "oops, I
>> typed the wrong thing" case, which I think is really the issue.
>
> Exactly, that's the common case where things go wrong. I guess using
> dwim_ref should be DWIM enough? :)
Hmm. Yeah, I mispoke before: I should have said dwim_ref instead of
resolve_ref (which doesn't dwym :) ).
Here's a sloppy patch that I think does what you want; but it might make
more sense to just iterate over ref_rev_parse_rules ourselves, as
dwim_ref does more than we care about (and we should probably
differentiate between "a branch already exists" and "this would make an
ambiguous ref").
---
diff --git a/branch.c b/branch.c
index d20fb04..409f445 100644
--- a/branch.c
+++ b/branch.c
@@ -122,6 +122,7 @@ void create_branch(const char *head,
unsigned char sha1[20];
char *real_ref, msg[PATH_MAX + 20];
struct strbuf ref = STRBUF_INIT;
+ char *junk;
int forcing = 0;
int len;
@@ -135,7 +136,8 @@ void create_branch(const char *head,
if (check_ref_format(ref.buf))
die("'%s' is not a valid branch name.", name);
- if (resolve_ref(ref.buf, sha1, 1, NULL)) {
+ if (dwim_ref(name, strlen(name), sha1, &junk)) {
+ free(junk);
if (!force)
die("A branch named '%s' already exists.", name);
else if (!is_bare_repository() && !strcmp(head, name))
^ permalink raw reply related
* Re: [PATCH] Introduce a filter-path argument to git-daemon, for doing custom path transformations
From: Johannes Schindelin @ 2009-03-12 16:50 UTC (permalink / raw)
To: Johan Sørensen; +Cc: git
In-Reply-To: <1236872914-43327-1-git-send-email-johan@johansorensen.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 575 bytes --]
Hi,
On Thu, 12 Mar 2009, Johan Sørensen wrote:
> The parameter for filter-path is an executable that will receive the service
> name, the client hostname and path to the repos the client requests as as
> arguments. It is then the responsibility of the script to return a zero
> terminated string on its stdout with the real path of the target repository.
>
> Signed-off-by: Johan Sørensen <johan@johansorensen.com>
I see that you addressed some, but not all of my concerns. Do you think
that I am wrong? Then please, by all means, increase my knowledge.
Ciao,
Dscho
^ permalink raw reply
* Re: git checkout -b origin/mybranch origin/mybranch
From: John Tapsell @ 2009-03-12 16:45 UTC (permalink / raw)
To: Jeff King; +Cc: Pieter de Bie, Sverre Rabbelier, Johannes Schindelin, Git List
In-Reply-To: <20090312163533.GA28205@coredump.intra.peff.net>
2009/3/12 Jeff King <peff@peff.net>:
> On Thu, Mar 12, 2009 at 04:16:09PM +0000, John Tapsell wrote:
>
>> I was thinking more along the lines of checking if it begins with
>> remotes/, origin/, tags/, stash/, bisect/ and blacklisting these.
>>
>> Can anyone suggest a reason that you really might want to create a
>> branch called origin/something ?
>
> The name "origin" is simply convention. So if you are thinking about
> blacklisting "origin/*", then it is certainly possible to have a false
> positive (although as you note, it is unlikely). But what is worse is
> that it is very likely for you to have a false negative if you use a
> different remote name (and people often do if they have multiple
> remotes).
>
> For example, in one of my projects where I do integration, "origin" is
> my own public repo, and I have a remote pointing to the public repo of a
> number of other developers from whom I pull. So I would encounter the
> same error by doing:
>
> git checkout -b mike/master mike/master
>
> but it would not be caught by your rule.
>
> One area where your rule _is_ nicer than mine is that mine only looks at
> what exists _now_ and doesn't future-proof you at all. So I could say
>
> git checkout -b origin/newtopic
>
> which might not be ambiguous. But if the remote adds a "newtopic"
> branch, then the next time I fetch it will _become_ ambiguous.
>
> Potentially you could blacklist "X/*" for every remote.X.* that
> exists in your config. Even that, of course, isn't future-proof against
> you creating a new remote. :)
>
> I think the future-proofing is probably not worth the effort. Catching
> things that are ambiguous _now_ will cover the "oops, I typed the wrong
> thing" case, which I think is really the issue.
Yep, makes sense.
I suppose you could do both. Blacklist and check if it already
exists, but I think just checking if it exists is "good enough".
John
^ permalink raw reply
* Re: git checkout -b origin/mybranch origin/mybranch
From: Pieter de Bie @ 2009-03-12 16:40 UTC (permalink / raw)
To: Jeff King; +Cc: John Tapsell, Sverre Rabbelier, Johannes Schindelin, Git List
In-Reply-To: <20090312163533.GA28205@coredump.intra.peff.net>
On Mar 12, 2009, at 4:35 PM, Jeff King wrote:
> I think the future-proofing is probably not worth the effort. Catching
> things that are ambiguous _now_ will cover the "oops, I typed the
> wrong
> thing" case, which I think is really the issue.
Exactly, that's the common case where things go wrong. I guess using
dwim_ref
should be DWIM enough? :)
- Pieter
^ permalink raw reply
* Re: [PATCH] read-tree A B C: do not create a bogus index and do not segfault
From: Linus Torvalds @ 2009-03-12 16:34 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, git, Jiri Olsa, Johannes Schindelin
In-Reply-To: <alpine.LNX.1.00.0903121024400.19665@iabervon.org>
On Thu, 12 Mar 2009, Daniel Barkalow wrote:
>
> I think one of the later refactorings may have given up on seeing
> conflicts while reading trees, but didn't drop the flag (perhaps because
> Linus knew at the time that my assumption that conflicts would actually
> have been recognized was false, and didn't realize that it was also the
> source of the flag).
That is very thoughtful of you, but I suspect the real reason was a much
simpler one: redoing the read-tree logic was a huge pain in the posterior,
and my primary goal at the time was to make the code more readable while
passing all the tests.
So while I tried to make patches minimal (which may well explain why the
flag remained), it wasn't the main goal, and trying to sort out the
read-tree logic into something more understandable was quite test-driven.
So I think the big issue to take away from this is that I think this is
such an odd case that if we want to support it, it would need explicit
tests. Or:
> I think it might be a good idea to take this as evidence that nobody is
> using read-tree with multiple trees without merge, and just disallow it.
Hmm. It _has_ been used. It's been useful for really odd things
historically, namely trying to merge different trees by hand. So while I
agree that we could probably remove it, it _is_ a very interesting
feature in theory, and since we have the code..
So I'd say "add a few tests for the known horrible cases" should be the
first approach. If it ever actually breaks again and becomes a big
maintenance headache, maybe _then_ remove the feature as not being worth
the pain?
Linus
^ permalink raw reply
* Re: git checkout -b origin/mybranch origin/mybranch
From: Jeff King @ 2009-03-12 16:35 UTC (permalink / raw)
To: John Tapsell
Cc: Pieter de Bie, Sverre Rabbelier, Johannes Schindelin, Git List
In-Reply-To: <43d8ce650903120916yb91113fy5485813c512c8108@mail.gmail.com>
On Thu, Mar 12, 2009 at 04:16:09PM +0000, John Tapsell wrote:
> I was thinking more along the lines of checking if it begins with
> remotes/, origin/, tags/, stash/, bisect/ and blacklisting these.
>
> Can anyone suggest a reason that you really might want to create a
> branch called origin/something ?
The name "origin" is simply convention. So if you are thinking about
blacklisting "origin/*", then it is certainly possible to have a false
positive (although as you note, it is unlikely). But what is worse is
that it is very likely for you to have a false negative if you use a
different remote name (and people often do if they have multiple
remotes).
For example, in one of my projects where I do integration, "origin" is
my own public repo, and I have a remote pointing to the public repo of a
number of other developers from whom I pull. So I would encounter the
same error by doing:
git checkout -b mike/master mike/master
but it would not be caught by your rule.
One area where your rule _is_ nicer than mine is that mine only looks at
what exists _now_ and doesn't future-proof you at all. So I could say
git checkout -b origin/newtopic
which might not be ambiguous. But if the remote adds a "newtopic"
branch, then the next time I fetch it will _become_ ambiguous.
Potentially you could blacklist "X/*" for every remote.X.* that
exists in your config. Even that, of course, isn't future-proof against
you creating a new remote. :)
I think the future-proofing is probably not worth the effort. Catching
things that are ambiguous _now_ will cover the "oops, I typed the wrong
thing" case, which I think is really the issue.
-Peff
^ permalink raw reply
* Re: git checkout -b origin/mybranch origin/mybranch
From: John Tapsell @ 2009-03-12 16:16 UTC (permalink / raw)
To: Jeff King; +Cc: Pieter de Bie, Sverre Rabbelier, Johannes Schindelin, Git List
In-Reply-To: <20090312153738.GA24690@coredump.intra.peff.net>
2009/3/12 Jeff King <peff@peff.net>:
> On Thu, Mar 12, 2009 at 03:21:48PM +0000, Pieter de Bie wrote:
>
>> You can also get this in other interactions, for example:
>>
>> $ git checkout -b origin/test HEAD
>> $ git checkout -b origin/test master
>>
>> yes, these might be user errors, but I still think it's not OK to create a
>> new ref 'refs/heads/origin/test' if there's also a 'refs/
>> remotes/origin/test' (as I've said a few months ago).
>
> One thing that has been missing from this discussion (and I think you
> are getting to it here) is a concrete rule for "X is harmful, and Y is
> not". That is, how do we know when to warn, and then what do we do?
>
> John's original example was "git checkout -b origin/test origin/test".
> So it's a problem that they're textually the same, but obviously there
> are more problematic cases.
>
> The behavior I think you are implying would be something like:
>
> When making origin/test, try to resolve_ref("origin/test"); if it
> fails, we are OK. If it succeeds, then we know we will be creating an
> ambiguity. Complain and refuse the creation unless "-f" is given.
>
> This would actually be a superset of the "branch already exists" case,
> so it should be pretty simple to code, and it makes for a simple rule:
> it is now "ref already exists".
+1
I was thinking more along the lines of checking if it begins with
remotes/, origin/, tags/, stash/, bisect/ and blacklisting these.
Can anyone suggest a reason that you really might want to create a
branch called origin/something ?
> Does that actually cover all of the problematic cases?
>
> -Peff
>
^ permalink raw reply
* [PATCH] Introduce a filter-path argument to git-daemon, for doing custom path transformations
From: Johan Sørensen @ 2009-03-12 15:48 UTC (permalink / raw)
To: git; +Cc: Johan Sørensen
In-Reply-To: <alpine.DEB.1.00.0903121218000.10279@pacific.mpi-cbg.de>
The parameter for filter-path is an executable that will receive the service
name, the client hostname and path to the repos the client requests as as
arguments. It is then the responsibility of the script to return a zero
terminated string on its stdout with the real path of the target repository.
Signed-off-by: Johan Sørensen <johan@johansorensen.com>
---
Documentation/git-daemon.txt | 15 +++++++++++
daemon.c | 54 +++++++++++++++++++++++++++++++++++++++++-
2 files changed, 68 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 36f00ae..bf8d31f 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -13,6 +13,7 @@ SYNOPSIS
[--strict-paths] [--base-path=path] [--base-path-relaxed]
[--user-path | --user-path=path]
[--interpolated-path=pathtemplate]
+ [--path-filter=executable]
[--reuseaddr] [--detach] [--pid-file=file]
[--enable=service] [--disable=service]
[--allow-override=service] [--forbid-override=service]
@@ -71,6 +72,20 @@ OPTIONS
After interpolation, the path is validated against the directory
whitelist.
+--path-filter=executable::
+ To support a more flexible directory layout a path filter script
+ can be used. The executable will receive the service name (upload-pack,
+ upload-archive or receive-pack), the client hostname and the request git
+ directory as arguments. The executable must return a zero-terminated
+ string on stdout which is the real path 'git-daemon' should serve. This
+ is useful when --interpolated-path doesn't buy you enough flexibility.
+ You could for instance keep support for old clone urls if you rename your
+ repository, or fetch a custom url-mapping from a third-party repo manager
+ application, or map deeply nested repository directories to a more
+ sensible layout for the outside world.
+ Please be aware that the executable spawned will have the same privileges
+ as the user you are running the git-daemon under.
+
--export-all::
Allow pulling from all directories that look like GIT repositories
(have the 'objects' and 'refs' subdirectories), even if they
diff --git a/daemon.c b/daemon.c
index d93cf96..e865e78 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "pkt-line.h"
#include "exec_cmd.h"
+#include "run-command.h"
#include <syslog.h>
@@ -22,6 +23,7 @@ static const char daemon_usage[] =
" [--strict-paths] [--base-path=path] [--base-path-relaxed]\n"
" [--user-path | --user-path=path]\n"
" [--interpolated-path=path]\n"
+" [--path-filter=path]\n"
" [--reuseaddr] [--detach] [--pid-file=file]\n"
" [--[enable|disable|allow-override|forbid-override]=service]\n"
" [--inetd | [--listen=host_or_ipaddr] [--port=n]\n"
@@ -58,6 +60,11 @@ static char *canon_hostname;
static char *ip_address;
static char *tcp_port;
+/* if defined, the script will be executed with the service name, hostname,
+ * and requested path on stdin and _must_ return with a successful exitcode
+ * and the new path on stdout */
+static char *path_filter_script;
+
static void logreport(int priority, const char *err, va_list params)
{
if (log_syslog) {
@@ -287,6 +294,42 @@ static int git_daemon_config(const char *var, const char *value, void *cb)
return 0;
}
+static char *run_path_filter_script(struct daemon_service *s, char *host,
+ char *dir) {
+ struct strbuf result_buf = STRBUF_INIT;
+ struct child_process filter_cmd;
+ const char *args[] = { path_filter_script, s->name, host, dir, NULL };
+
+ loginfo("Executing path filter script: '%s %s %s %s'",
+ path_filter_script, s->name, host, dir);
+ memset(&filter_cmd, 0, sizeof(filter_cmd));
+ filter_cmd.argv = args;
+ filter_cmd.out = -1;
+
+ if (start_command(&filter_cmd)) {
+ logerror("path filter: unable to fork path_filter_script");
+ return dir;
+ }
+
+ if (strbuf_read(&result_buf, filter_cmd.out, PATH_MAX) < 0) {
+ strbuf_release(&result_buf);
+ close(filter_cmd.out);
+ logerror("path filter: script read returned %s", strerror(errno));
+ return dir;
+ }
+
+ close(filter_cmd.out);
+ if (finish_command(&filter_cmd)) {
+ logerror("path filter script died with strange error");
+ return dir;
+ }
+
+ if (result_buf.len > 0)
+ dir = strbuf_detach(&result_buf, NULL);
+
+ return dir;
+}
+
static int run_service(char *dir, struct daemon_service *service)
{
const char *path;
@@ -557,7 +600,12 @@ static int execute(struct sockaddr *addr)
* Note: The directory here is probably context sensitive,
* and might depend on the actual service being performed.
*/
- return run_service(line + namelen + 5, s);
+ if (path_filter_script) {
+ return run_service(run_path_filter_script(s, hostname,
+ line + namelen + 5), s);
+ } else {
+ return run_service(line + namelen + 5, s);
+ }
}
}
@@ -1018,6 +1066,10 @@ int main(int argc, char **argv)
pid_file = arg + 11;
continue;
}
+ if (!prefixcmp(arg, "--path-filter=")) {
+ path_filter_script = arg + 14;
+ continue;
+ }
if (!strcmp(arg, "--detach")) {
detach = 1;
log_syslog = 1;
--
1.6.1
^ permalink raw reply related
* Re: newb: Given a commit id, find which branches have it as an ancestor
From: Johannes Sixt @ 2009-03-12 15:38 UTC (permalink / raw)
To: Kelly F. Hickel; +Cc: git
In-Reply-To: <63BEA5E623E09F4D92233FB12A9F794302E0F98D@emailmn.mqsoftware.com>
$ git branch -a --contains the-sha1
-- Hannes
^ permalink raw reply
* Re: git checkout -b origin/mybranch origin/mybranch
From: Jeff King @ 2009-03-12 15:37 UTC (permalink / raw)
To: Pieter de Bie
Cc: Sverre Rabbelier, John Tapsell, Johannes Schindelin, Git List
In-Reply-To: <B9479687-D936-4EE1-A5A4-663968B76EBF@ai.rug.nl>
On Thu, Mar 12, 2009 at 03:21:48PM +0000, Pieter de Bie wrote:
> You can also get this in other interactions, for example:
>
> $ git checkout -b origin/test HEAD
> $ git checkout -b origin/test master
>
> yes, these might be user errors, but I still think it's not OK to create a
> new ref 'refs/heads/origin/test' if there's also a 'refs/
> remotes/origin/test' (as I've said a few months ago).
One thing that has been missing from this discussion (and I think you
are getting to it here) is a concrete rule for "X is harmful, and Y is
not". That is, how do we know when to warn, and then what do we do?
John's original example was "git checkout -b origin/test origin/test".
So it's a problem that they're textually the same, but obviously there
are more problematic cases.
The behavior I think you are implying would be something like:
When making origin/test, try to resolve_ref("origin/test"); if it
fails, we are OK. If it succeeds, then we know we will be creating an
ambiguity. Complain and refuse the creation unless "-f" is given.
This would actually be a superset of the "branch already exists" case,
so it should be pretty simple to code, and it makes for a simple rule:
it is now "ref already exists".
Does that actually cover all of the problematic cases?
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions
From: martin f krafft @ 2009-03-12 15:26 UTC (permalink / raw)
To: Uwe Kleine-König, Brian Campbell, Junio C Hamano, git
In-Reply-To: <20090312152039.GA15626@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 781 bytes --]
also sprach Uwe Kleine-König <u.kleine-koenig@pengutronix.de> [2009.03.12.1620 +0100]:
> IMHO we should reuse as much as possible from git.git. For me even
> requiring a git.git checkout to use its files would be OK. I consider
> that even better then duplicating the relevant files.
Maybe we could even start to think about integrating TopGit back
into git.git?
--
martin | http://madduck.net/ | http://two.sentenc.es/
"perhaps debian is concerned more about technical excellence rather
than ease of use by breaking software. in the former we may excel.
in the latter we have to concede the field to microsoft. guess
where i want to go today?"
-- manoj srivastava
spamtraps: madduck.bogus@madduck.net
[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: git checkout -b origin/mybranch origin/mybranch
From: Pieter de Bie @ 2009-03-12 15:21 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: John Tapsell, Johannes Schindelin, Git List
In-Reply-To: <fabb9a1e0903120643r3cfefdfej560ff7edda2be6f5@mail.gmail.com>
On Mar 12, 2009, at 1:43 PM, Sverre Rabbelier wrote:
> The point is that we _already_ warned the user (like Dscho pointed
> out), and that(as you pointed out), it didn't work :P.
You can also get this in other interactions, for example:
$ git checkout -b origin/test HEAD
$ git checkout -b origin/test master
yes, these might be user errors, but I still think it's not OK to
create a new ref 'refs/heads/origin/test' if there's also a 'refs/
remotes/origin/test' (as I've said a few months ago).
^ permalink raw reply
* newb: Given a commit id, find which branches have it as an ancestor
From: Kelly F. Hickel @ 2009-03-12 15:21 UTC (permalink / raw)
To: git; +Cc: Kelly F. Hickel
Hi all, I've been working on testing importing our cvs repo via cvs2git,
then using cvsps to pull incremental updates. Something seems to have
gone awry with one of the commits, and I'm having trouble tracking it
down.
This is a question about how to track something down after the fact, not
a question about what went wrong with the cvsps import....
This is git 1.6.1 running on Centos 5.2 linux.
So, the scenario is that one of the last few commits pulled into my git
repo by cvsps/cvsimport should have landed on origin/master, but when I
look at the file, the change is missing. I'm trying to figure out
"where it went", since it didn't go where I expected it.
Things I've tried that didn't tell me what I wanted to know:
$ git name-rev 15fa81b
15fa81b undefined
$ git log --children 15fa81b
This shows me a bunch of commits that, going by the commit date, appear
to be ancestors of the commit I'm interested in, not children.
$ git checkout 15fa81b5ae
Note: moving to "15fa81b5ae" which isn't a local branch If you want to
create a new branch from this checkout, you may do so (now or later) by
using -b with the checkout command again. Example:
git checkout -b <new_branch_name>
HEAD is now at 15fa81b... Changed version to 4.1.0.157 $ gitk (as
expected, shows me that the commit I care about is the latest in the
workspace)
$ git checkout master
Previous HEAD position was 15fa81b... Changed version to 4.1.0.157
Switched to branch "master"
$ gitk
Doesn't list my target commit, in fact, doesn't list any commits after
the cvs2git date, so it appears that none of my cvsps pulled commits
landed on master (ok, so maybe this post is about what went wrong, just
a little ;-} ).
I suspect that I'm missing some factoid in trying to map my workflow to
Git, but this seems like the kind of thing I'd want to know, i.e. given
a commit, what branches have that commit as an ancestor. It would seem
to be useful in two cases:
1) I've found a commit that introduced a bug and want to know what
releases that bug ended up in.
2) I've identified a fix for a previous bug and want to know what
releases already contain the fix.
(ok, those are pretty much the same workflow, but different reasons).
What am I missing????
--
Kelly F. Hickel
Senior Product Architect
MQSoftware, Inc.
952-345-8677 Office
952-345-8721 Fax
kfh@mqsoftware.com
www.mqsoftware.com
Certified IBM SOA Specialty
Your Full Service Provider for IBM WebSphere
Learn more at www.mqsoftware.com
^ permalink raw reply
* Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions
From: Uwe Kleine-König @ 2009-03-12 15:20 UTC (permalink / raw)
To: Brian Campbell, Junio C Hamano; +Cc: git, Petr Baudis
In-Reply-To: <78BA729B-0026-45D0-96FC-330700519AAB@dartmouth.edu>
Hello Brian, hello Junio,
On Thu, Mar 12, 2009 at 11:00:00AM -0400, Brian Campbell wrote:
> On Mar 12, 2009, at 3:45 AM, Uwe Kleine-König wrote:
>
>>> - You will be utterly confused by a local branch whose name is
>>> "refs/top-bases/foo"
>> You mean a branch that has the full name refs/heads/refs/top-bases/
>> foo?
>> Well OK, valid concern.
>
> Yes, you're right, this is a problem.
>
>>> To fix these, you might want to do something like:
>>>
>>> if head_=$(git symbolic-ref HEAD)
Shouldn't git symbolic-ref -q HEAD be used here?
>>> then
>>> case "$head_" in
>>> refs/heads/*)
>>> echo "${head_#refs/heads/}"
>>> ;;
>>> refs/top-bases/*)
>>> echo "${head_#refs/top-bases/}"
>>> ;;
>>> *)
>>> echo "$head_"
>>> ;;
>>> esac
>>> else
>>> whatever you want to do on a detached HEAD
How do I distinguish between a detached HEAD and another error? I have
the feeling that git symbolic-ref -q HEAD should exit(0) with a detached
HEAD.
>> Thanks Junio and Brian.
>>
>> Brian, do you update the series?
>
> Sure, I'll send an updated patch.
>
> I'm thinking that for the detached HEAD case, this function should die
> with a message about not being on a valid branch, and then the call site
> in tg-summary that doesn't care about being on a valid branch should
> ignore the error and leave curname empty. Does that sound about right?
mmh, I would return "" and let the caller handle that.
> Also, has anyone considered writing a test suite for TopGit?
Yes, but I didn't found the time for that until now. If you'd volunteer
that would be very welcome.
IMHO we should reuse as much as possible from git.git. For me even
requiring a git.git checkout to use its files would be OK. I consider
that even better then duplicating the relevant files.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Strasse 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions
From: Brian Campbell @ 2009-03-12 15:00 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: Junio C Hamano, git, Petr Baudis
In-Reply-To: <20090312074524.GA14844@pengutronix.de>
On Mar 12, 2009, at 3:45 AM, Uwe Kleine-König wrote:
>> - You will be utterly confused by a local branch whose name is
>> "refs/top-bases/foo"
> You mean a branch that has the full name refs/heads/refs/top-bases/
> foo?
> Well OK, valid concern.
Yes, you're right, this is a problem.
>> To fix these, you might want to do something like:
>>
>> if head_=$(git symbolic-ref HEAD)
>> then
>> case "$head_" in
>> refs/heads/*)
>> echo "${head_#refs/heads/}"
>> ;;
>> refs/top-bases/*)
>> echo "${head_#refs/top-bases/}"
>> ;;
>> *)
>> echo "$head_"
>> ;;
>> esac
>> else
>> whatever you want to do on a detached HEAD
>> fi
> Thanks Junio and Brian.
>
> Brian, do you update the series?
Sure, I'll send an updated patch.
I'm thinking that for the detached HEAD case, this function should die
with a message about not being on a valid branch, and then the call
site in tg-summary that doesn't care about being on a valid branch
should ignore the error and leave curname empty. Does that sound about
right? I'm fairly new to doing Bourne shell scripting, so I don't yet
have a good sense of how these things should be structured.
Also, has anyone considered writing a test suite for TopGit? I
actually got fairly deep in to a series of 10 or so patches before I
hit these problems, since tg-create worked fine as long as I only
supplied one dependency, and I didn't notice the second issue until I
tried to do a tg-update after modifying one of my base patches.
-- Brian
^ permalink raw reply
* Re: [PATCH 2/2] [TopGit] Portability: Don't use alternation ("|") in sed regular expressions
From: Uwe Kleine-König @ 2009-03-12 15:09 UTC (permalink / raw)
To: Brian Campbell; +Cc: git, Petr Baudis
In-Reply-To: <1236837389-35687-2-git-send-email-brian.p.campbell@dartmouth.edu>
Hello Brian,
one more minor nitpick:
On Thu, Mar 12, 2009 at 01:56:29AM -0400, Brian Campbell wrote:
> -[ -n "$name" ] || name="$(git symbolic-ref HEAD | sed 's#^refs/\(heads\|top-bases\)/##')"
> +[ -n "$name" ] || name="$(current_branch)"
current_branch doesn't look like the correct name. I'd prefer
current_tgpatch is better?! (For current_branch I would expect that
only refs/heads is stripped.)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH] read-tree A B C: do not create a bogus index and do not segfault
From: Daniel Barkalow @ 2009-03-12 14:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jiri Olsa, Johannes Schindelin, Linus Torvalds
In-Reply-To: <7v3adjjj1y.fsf_-_@gitster.siamese.dyndns.org>
On Thu, 12 Mar 2009, Junio C Hamano wrote:
> "git read-tree A B C..." without the "-m" (merge) option is a way to read
> these trees on top of each other to get an overlay of them.
>
> An ancient commit ee6566e (Rewrite read-tree, 2005-09-05) passed the
> ADD_CACHE_SKIP_DFCHECK flag when calling add_index_entry() to add the
> paths obtained from these trees to the index, but it is an incorrect use
> of the flag. The flag is meant to be used by callers who know the
> addition of the entry does not introduce a D/F conflict to the index in
> order to avoid the overhead of checking.
I think that this implementation actually thought it knew that the entry
couldn't cause a D/F conflict. (Of course, it turned out 2.5 years later
that it was wrong, on account of missing an implication of the way the
index is sorted.)
> This bug resulted in a bogus index that records both "x" and "x/z" as a
> blob after reading three trees that have paths ("x"), ("x", "y"), and
> ("x/z", "y") respectively. 34110cd (Make 'unpack_trees()' have a separate
> source and destination index, 2008-03-06) refactored the callsites of
> add_index_entry() incorrectly and added more codepaths that use this flag
> when it shouldn't be used.
>
> Also, 0190457 (Move 'unpack_trees()' over to 'traverse_trees()' interface,
> 2008-03-05) introduced a bug to call add_index_entry() for the tree that
> does not have the path in it, passing NULL as a cache entry. This caused
> reading multiple trees, one of which has path "x" but another doesn't, to
> segfault.
I think one of the later refactorings may have given up on seeing
conflicts while reading trees, but didn't drop the flag (perhaps because
Linus knew at the time that my assumption that conflicts would actually
have been recognized was false, and didn't realize that it was also the
source of the flag).
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> * I suspect that we can probably remove SKIP_DFCHECK option by fixing
> tree.c::read_tree(); the only caller the big comment at the beginning
> of it talks about is overlay_tree_on_cache() in builtin-ls-files.c and
> we haven't gained any new callers of the function.
>
> It is a bit sad that a very good looking refactoring and rewriting
> patch introduced this kind of regression that has gone unnoticed for a
> long time. I managed to point three fingers and they turn out to be
> all ancient commits before v1.5.5.
I'm not too surprised that "read-tree" with multiple trees without -m
doesn't actually work, because I don't think this has ever been useful for
anything, and nobody's been able to come up with an implementation
of read-tree that's particularly easy to understand. So regular testing
wouldn't find this problem, and inspection wouldn't find it, either.
I think it might be a good idea to take this as evidence that nobody is
using read-tree with multiple trees without merge, and just disallow it.
If it takes a year to hit a pretty big bug in a code path, and using the
code path was due to a typo anyway, it's unlikely that the code path is
actually used for anything.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: Choosing between "renaming" and "copy"
From: Andreas Ericsson @ 2009-03-12 14:44 UTC (permalink / raw)
To: Samuel Tardieu; +Cc: git
In-Reply-To: <2009-03-12-14-01-15+trackit+sam@rfc1149.net>
Samuel Tardieu wrote:
> I renamed a file in my repository, and made a slightly modified copy
> of it. It looks like GIT gets confused on which one is the renaming
> and which one is the copy, and doesn't favour the 100% identical one
> to be chosen as the renaming.
>
> Not a big deal, but maybe git could be more clever here.
>
> % git commit -m "Split into flash and ram alternatives."
> [stm32-sk 601462c] Split into flash and ram alternatives.
> 3 files changed, 3 insertions(+), 2 deletions(-)
> copy Demo/CORTEX_STM32SK_GCC/{stm32f103r8t6.ld => stm32f103r8t6_flash.ld} (100%)
> rename Demo/CORTEX_STM32SK_GCC/{stm32f103r8t6.ld => stm32f103r8t6_ram.ld} (98%)
>
There isn't that much more to be clever about, really. One is a rename+edit,
the other is a copy. The other way around would have been copy+edit + rename
which isn't necessarily an improvement.
Looking at how git internally[1] does things and remembering the meanings of
"copy" and "rename" though, it makes perfect sense to leave it as-is.
[1].
In git, the content is part of the (object) name, so changing the content
makes it closer to a rename than a copy, while changing the location always
makes it a copy, although sometimes coupled with a delete.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.
^ permalink raw reply
* Re: git checkout -b origin/mybranch origin/mybranch
From: John Tapsell @ 2009-03-12 14:14 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Johannes Schindelin, Git List
In-Reply-To: <fabb9a1e0903120643r3cfefdfej560ff7edda2be6f5@mail.gmail.com>
2009/3/12 Sverre Rabbelier <srabbelier@gmail.com>:
> Heya,
>
> On Thu, Mar 12, 2009 at 14:18, John Tapsell <johnflux@gmail.com> wrote:
>> Is probably a mistake by the user. We should warn the user and point
>> them in the right direction.
>
> The point is that we _already_ warned the user (like Dscho pointed
> out), and that(as you pointed out), it didn't work :P.
Just doing:
git branch -b origin/master origin/master
gives no error or warning at all.
John
>
> --
> Cheers,
>
> Sverre Rabbelier
>
^ permalink raw reply
* Re: git doc build failure on OS X 10.5.6 (Leopard) during xmlto phase
From: Alejandro Riveira Fernández @ 2009-03-12 14:02 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Jay Soffian, Tom Holaday
In-Reply-To: <49B8EF3E.2070208@drmicha.warpmail.net>
El Thu, 12 Mar 2009 12:17:18 +0100
Michael J Gruber <git@drmicha.warpmail.net> escribió:
> Alejandro Riveira venit, vidit, dixit 11.03.2009 21:12:
> >>
> > "Me too" from a Ubuntu 8.10 Box
>
> Following up on this:
> On Fedora 10, I have asciidoc 8.2.5 and docbook 1.7.4 xsl's. For proper
> man and html doc, I have to set DOCBOOK_XSL_172=Yes but leave ASCIIDOC8
> unset! I always forget, though (just like the packagers).
>
> Setting DOCBOOK_XSL_172 shuts off a certain hack which would otherwise
> introduce the notorious .ft in man output.
>
> Setting ASCIIDOC8 would keep _emphasis_ from being transformed into
> <emphasis>emphasis</emphasis>, which means it would end up as literal
> _emphasis_ in man as well as html.
>
> Michael
>
> BTW: Alejandro, please don't cull cc here.
I'm sorry :[
In my defense that was using gmane new service via Pan
Thanks for the explanation on how to workaround the issue
^ permalink raw reply
* Re: git checkout -b origin/mybranch origin/mybranch
From: Sverre Rabbelier @ 2009-03-12 13:43 UTC (permalink / raw)
To: John Tapsell; +Cc: Johannes Schindelin, Git List
In-Reply-To: <43d8ce650903120618h79686207vaa478c54f34e26f8@mail.gmail.com>
Heya,
On Thu, Mar 12, 2009 at 14:18, John Tapsell <johnflux@gmail.com> wrote:
> Is probably a mistake by the user. We should warn the user and point
> them in the right direction.
The point is that we _already_ warned the user (like Dscho pointed
out), and that(as you pointed out), it didn't work :P.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: git checkout -b origin/mybranch origin/mybranch
From: John Tapsell @ 2009-03-12 13:18 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git List
In-Reply-To: <alpine.DEB.1.00.0903121357320.6335@intel-tinevez-2-302>
2009/3/12 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> Hi,
>
> On Thu, 12 Mar 2009, John Tapsell wrote:
>
>> 2009/3/12 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>>
>> > On Thu, 12 Mar 2009, John Tapsell wrote:
>> >
>> >> One of my collegues did:
>> >>
>> >> git checkout origin/somebranch
>> >>
>> >> git complained that they need to specify the name with -b. So they did:
>> >>
>> >> git checkout -b origin/somebranch origin/somebranch
>> >
>> > Yeah, a pilot error. It should have been
>> >
>> > $ git checkout -t origin/somebranch
>>
>> Maybe the error message for "git checkout origin/somebranch" should
>> suggest: git checkout -t origin/somebranch?
>>
>> > I have to wonder, though, why "git checkout origin/somebranch" did not
>> > detach your HEAD.
>>
>> It did. But that doesn't affect doing "git checkout -b
>> origin/somebranch origin/somebranch" afterwards.
>
> So did the warning read something like this?
>
> -- snip --
> moving to "d36a18dc9cdf1dfce8632e42491b826387aa3230" which isn't a local
> branch
> If you want to create a new branch from this checkout, you may do so
> (now or later) by using -b with the checkout command again. Example:
> git checkout -b <new_branch_name>
> -- snap --
>
> ?
I'm really not sure what point you're trying to prove. Yes, we all
know that the user made a mistake in thinking that the new_branch_name
should be "origin/mybranch".
What I'm suggesting is that git tries to not let the user shoot
himself in the foot so easily.
I'm saying that:
git checkout -b origin/mybranch origin/mybranch
Is probably a mistake by the user. We should warn the user and point
them in the right direction.
John
^ 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