* Re: What's in git.git (part #2)
From: Johannes Schindelin @ 2006-06-06 8:19 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20060606053905.GA9797@spearce.org>
Hi,
On Tue, 6 Jun 2006, Shawn Pearce wrote:
> *1* Generating a simple pack with only deflate compression on the
> objects should be trivial. Since this is 100% pure Java code nobody
> should be expecting great performance out of it from day 1 anyway.
If you use jzlib (http://www.jcraft.com/jzlib/) it should not be slow.
IMHO the I/O will be the bottleneck, but I would be happy to be
contradicted by the facts.
Ciao,
Dscho
^ permalink raw reply
* Re: New release?
From: Johannes Schindelin @ 2006-06-06 8:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Linus Torvalds
In-Reply-To: <7vodx6zus2.fsf@assigned-by-dhcp.cox.net>
Hi,
On Mon, 5 Jun 2006, Junio C Hamano wrote:
> - letting fetch-pack ask for an arbitrary commit object the
> user obtained out of band (Eric W Biederman) -- waiting for
> updated patch. We would need a corresponding one-liner patch
> to upload-pack when we do this.
>
> This can wait.
I think that this could be an important step towards a sensible "shallow
clone": we could introduce "remote alternates", where sha1_file
transparently loads (and stores) single commit objects and tree objects
together with their subtrees and blobs from a remote, if they are not
present in the shallow repo yet.
This will be blazingly _slow_, I guess, if one does any such stupid thing
as git-fsck-objects... However, it should be fixable (expensive commands,
i.e. commands traversing extensive history/object lists, could just barf
if the repo is shallow).
> - using ~/.gitrc to give a fall-back default when
> $GIT_DIR/config does not have values.
>
> I suspect this would be more involved than Pasky's initial
> patch; but it can wait.
I think that this is quite important for the aliases to be useful.
However, it does not blend well with the mmap()ifying of the config
reading.
Also, there is the problem of unique keys. You want unique keys to be only
unique _per config file_, i.e. the local config can override the user's
config. This is probably only relevant for git-repo-config, though.
Ciao,
Dscho
^ permalink raw reply
* [RFC] revision limiter in git-rev-list
From: Marco Costalba @ 2006-06-06 8:36 UTC (permalink / raw)
To: git; +Cc: junkio
We currently can run something like
git-rev-list --topo-order --parents HEAD -- foo1.c foo2.c foo3.c
And have in output a list of revisions that modify paths foo1.c,
foo2.c and foo.3
For each revision we have also the corresponding pseudo-parents,
i.e. the proper parents chosen among the revisions in output list.
The idea is to extend this behaviour to also commit objects sha.
As example, given the following revisions history:
a
b-
| c
| d
e
f
We could add a new option --filtered so that
git-rev-list --topo-order --filtered HEAD -- a d e
Gives the following
a
b-
| d
e
Note that the merge point b has been added implicitly as in path limiter case.
This is a powerful and quite general option that can be use for a large
and interesting number of cases.
1) Get the branch a given <sha> belongs
git-rev-list --topo-order --parents --filtered
HEAD -- <sha> branchList[0] branchList[1]... branchList[k-1]
where branchList[] is the vector of branches of lenght k
Searched branch is the one where sha appears as one of his parents.
2) Get nearest previous tag of a given <sha>
git-rev-list --topo-order --parents -n1 --filtered
<sha> -- <sha> tagList[0] tagList[1]... tagList[n-1]
where tagList[] is the vector of tags of lenght n
In output we have only one revision (see option -n1) that is <sha>,
his parent(s) are the nearest tag(s).
3) Get tag/branch history with a sensible graph (using a GUI frontend)
git-rev-list --topo-order --parents --filtered HEAD -- tagList[0] ...
branchList[n-1]
git-rev-list --topo-order --parents --filtered HEAD -- branchList[0]
... branchList[n-1]
4) Filter/find revisions according to a given search criteria (using a
GUI frontend)
git-rev-list --topo-order --parents --filtered HEAD -- matchList[0]
... matchList[n-1]
where matchList[] is the vector of sha's matching a filter criteria and could
be provided by a GUI frontend that already normally have filter capabilities.
The plus here is to let frontend to draw a sensible graph of the
resulting revisions.
Probably there are other uses of this option that is very powerful
because essentially
it adds topological information to a given set of revisions.
Of course I really ignore any implementation difficult/feasibility issues ;-)
Marco
^ permalink raw reply
* Re: New release?
From: Junio C Hamano @ 2006-06-06 10:27 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Linus Torvalds
In-Reply-To: <Pine.LNX.4.63.0606061019440.11478@wbgn013.biozentrum.uni-wuerzburg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Mon, 5 Jun 2006, Junio C Hamano wrote:
>
>> - letting fetch-pack ask for an arbitrary commit object the
>> user obtained out of band (Eric W Biederman) -- waiting for
>> updated patch. We would need a corresponding one-liner patch
>> to upload-pack when we do this.
>>
>> This can wait.
>
> I think that this could be an important step towards a sensible "shallow
> clone"...
I did not say we are not interested in doing this ever. The
"arbitrary commit" thing is easy but I do not think it is all
that important to hold all the good stuff back that happened
since 1.3.0 and delay 1.4.0.
Also, what you talk about the "lazy clone" is a lot more
involved than what Eric wanted to have. It is _never_ shallow
clones that normal users would want -- making it shallow to
explicitly say certain operations break is a cop-out for
implementors. What the users really want is to be in total
control -- ranging from completely on-demand ala CVS and SVN to
"down to this commit in the history I would want to be cached on
the local machine so that I can go offline and still do useful
things with the history", with new failure modes for history
traversing commands to exit gracefully when offline. That _is_
the ideal but I know it won't be within reach anytime soon.
>> - using ~/.gitrc to give a fall-back default when
>> $GIT_DIR/config does not have values.
>>
>> I suspect this would be more involved than Pasky's initial
>> patch; but it can wait.
>
> I think that this is quite important for the aliases to be useful.
I am not saying this is unimportant. Again, however, it is not
that important to hold other good stuff.
^ permalink raw reply
* Re: [RFC] revision limiter in git-rev-list
From: Junio C Hamano @ 2006-06-06 10:29 UTC (permalink / raw)
To: Marco Costalba; +Cc: git, junkio
In-Reply-To: <e5bfff550606060136l59143ef2mdb9dc68ab78e9ff1@mail.gmail.com>
"Marco Costalba" <mcostalba@gmail.com> writes:
> Of course I really ignore any implementation difficult/feasibility issues ;-)
I do not think your problem is ignoring difficulty/feasibility.
The bigger problem is ignoring to describe the semantics of what
you are proposing.
For example:
> As example, given the following revisions history:
>
> a
> b-
> | c
> | d
> e
> f
>
> We could add a new option --filtered so that
>
> git-rev-list --topo-order --filtered HEAD -- a d e
>
> Gives the following
>
> a
> b-
> | d
> e
Why does it give that? Where is the HEAD in the example, and
how does it affect the result? I presume when you wrote the
unfiltered graph, you meant time flows from top to bottom. In
other words, I would have written the original graph like this,
time flowing from left to right:
c---d
/
a---b---e---f
Does your rev-list given above give different results depending
on HEAD being d or f? If so how, and for which case is the
example output produced? If not, what significance does the
HEAD argument have in the algorithm that generates the result
you are proposing (maybe your example had time flowing from
bottom to top, then the HEAD might be at a, and you did not have
to think about this question when you wrote the message)? These
are rhetorical questions, so do not waste time thinking about
answers to them before reading to the end of this message.
> Note that the merge point b has been added implicitly as in
> path limiter case.
I do not think path limiter case adds anything. A merge is
shown if it touches the path in an nontrivial way, but otherwise
it isn't. Also b is not a merge unless time is flowing from
bottom to top in your picture -- it is a fork point.
While I really do not think this belongs to rev-list, I suspect
what you want is a command that takes a set of commits you are
interested in and gives you an abbreviated topology across them.
I think it might be a good thing to have in our toolset (didn't
I say that already?).
So your example would become:
Given this graph (and there may be other nodes before a or after
d or f):
c---d
/
a---b---e---f
the user is interested in A, D, and E. Show an
abbreviated topology containing them.
which would give you
D
/
A---B---E
BTW, interestingly enough if the time flowed from right to left,
the answer is the same here.
The command line would not involve HEAD unless it is also one of
the commits you are interested in. So it would be something
like:
git graph-reduce A D E
which would give you:
E B
D B
B A
or even (since you are interested in reachability):
E B A
D B A
B A
Unfortunately, your description is a bit too fuzzy to me, so I
am making guesses as to what you really want. For example,
although you said "b is included because it is a merge", I
strongly suspect you have cases where you would want to and not
want to include a fork point or a merge point in the result,
depending on the commits you are interested in. If you are
reducing this graph for A, H, and J:
f---g
/ \
c---d---e---h
/
a---b---i---j
I think you would want to see this as the result of graph
reduction:
H
/
A---B---J
instead of:
C---E---H
/
A---B---J
That is, although e is a merge and c is a fork point, they do
not bring anything interesting in the bird's eye view picture,
and are filtered out. However, b is a point where lines of
development leading to H and J (two of the commits the user is
interested in) forks, and it is interesting.
Is this kind of graph reduction what you are after?
^ permalink raw reply
* Re: [RFC] revision limiter in git-rev-list
From: Marco Costalba @ 2006-06-06 11:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7j3uvapa.fsf@assigned-by-dhcp.cox.net>
On 6/6/06, Junio C Hamano <junkio@cox.net> wrote:
> "Marco Costalba" <mcostalba@gmail.com> writes:
>
> > Of course I really ignore any implementation difficult/feasibility issues ;-)
>
> I do not think your problem is ignoring difficulty/feasibility.
> The bigger problem is ignoring to describe the semantics of what
> you are proposing.
>
Sadly I have to agree, sorry.
> For example:
>
> > As example, given the following revisions history:
> >
> > a
> > b-
> > | c
> > | d
> > e
> > f
> >
> > We could add a new option --filtered so that
> >
> > git-rev-list --topo-order --filtered HEAD -- a d e
> >
> > Gives the following
> >
> > a
> > b-
> > | d
> > e
>
> Why does it give that? Where is the HEAD in the example,
HEAD is "a", just to try to be more clear that's the graph you would see running
gitk HEAD: HEAD is at top ("a") initial revision is at bottom ("e").
>
> > Note that the merge point b has been added implicitly as in
> > path limiter case.
>
> I do not think path limiter case adds anything. A merge is
> shown if it touches the path in an nontrivial way, but otherwise
> it isn't.
Yes.
>Also b is not a merge unless time is flowing from
> bottom to top in your picture -- it is a fork point.
>
I meant a graph as shown by gitk HEAD, so I really meant
"b" is a merge.
> While I really do not think this belongs to rev-list, I suspect
> what you want is a command that takes a set of commits you are
> interested in and gives you an abbreviated topology across them.
> I think it might be a good thing to have in our toolset (didn't
> I say that already?).
>
> So your example would become:
>
> Given this graph (and there may be other nodes before a or after
> d or f):
>
> c---d
> /
> a---b---e---f
>
> the user is interested in A, D, and E. Show an
> abbreviated topology containing them.
>
> which would give you
>
> D
> /
> A---B---E
>
Yes.
>
> Unfortunately, your description is a bit too fuzzy to me, so I
> am making guesses as to what you really want. For example,
> although you said "b is included because it is a merge", I
> strongly suspect you have cases where you would want to and not
> want to include a fork point or a merge point in the result,
> depending on the commits you are interested in. If you are
> reducing this graph for A, H, and J:
>
> f---g
> / \
> c---d---e---h
> /
> a---b---i---j
>
> I think you would want to see this as the result of graph
> reduction:
>
> H
> /
> A---B---J
>
> instead of:
>
> C---E---H
> /
> A---B---J
>
> That is, although e is a merge and c is a fork point, they do
> not bring anything interesting in the bird's eye view picture,
> and are filtered out. However, b is a point where lines of
> development leading to H and J (two of the commits the user is
> interested in) forks, and it is interesting.
>
> Is this kind of graph reduction what you are after?
>
>
Practically speaking it's the kind of reduction for whom examples I
gave _do_ work.
Marco
^ permalink raw reply
* Re: [RFC] revision limiter in git-rev-list
From: Marco Costalba @ 2006-06-06 11:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7j3uvapa.fsf@assigned-by-dhcp.cox.net>
>
> While I really do not think this belongs to rev-list, I suspect
> what you want is a command that takes a set of commits you are
> interested in and gives you an abbreviated topology across them.
I was thinking at an extension of git-rev-list because
1) Current git-rev-list options are quite orthogonal with rev limiter.
As example in previous given examples -n and --parents options are
used and I think more could be used with
interesting results.
Current git-rev-list options are a lot and are very powerful, it's a
pity if this new feature do not inherit them.
2) This feature could be seen as a generalization of path limiting.
>From today:
git-rev-list HEAD -- <path 1> <path 2> ... <path n>
To possible:
git-rev-list HEAD -- <obj 1> <obj 2> ... <obj n>
Where obj == <path> || obj == <commit sha> || obj == <something else
I didn't think of>
Of course we need a (syntactic) way to disambiguate the arguments
after '--' but the results are very powerful and general, as example
we could mix commit objects _and_ paths in git-rev-list command line
(git-rev-list HEAD -- foo.c tag1) and also be able to use the full
set of git-rev-list options before the '--'
Marco
^ permalink raw reply
* [ANNOUNCE] tig 0.4
From: Jonas Fonseca @ 2006-06-06 12:18 UTC (permalink / raw)
To: git
Hello,
Too many improvements to keep me longer from releasing version 0.4 of
tig, the text-mode interface for git. First off, a big thanks to Aneesh
Kumar for helping with improving the basic user interface. Among the
highlights, colors are now configurable in ~/.tigrc and the diff and
main views are better integrated. For more info, changes I feel are
most important can be found below.
I hope to focus more on fixing some of the accumulated bugs for tig-0.5.
A particular embarrassing one, is the problem with text wrapping for
long lines which can end up corrupting the screen. But there should also
be room for a first shot at displaying revision graph and maybe
configurable keybindings.
A list of tig resources:
- Homepage: http://jonas.nitro.dk/tig/
- Tarballs: http://jonas.nitro.dk/tig/releases/
- Git URL: http://jonas.nitro.dk/tig/tig.git
- Gitweb: http://pasky.or.cz/gitweb.cgi?p=tig.git;a=summary
Tarballs contain all the generated documentation, as do the #release
branch of the git repository. Thanks to pasky for the gitweb hosting.
---
Jonas Fonseca:
Redraw the whole display after toggling line number
Bind quit to 'Q'
Add close view request; bound to 'q' by default
Add preliminary support for UTF-8 handling in the main view
Make Enter in the main view switch to the split diff view
Pressing Enter in the diff view will now scroll it one line down
Bind '-' to PageUp; raises Mutt compatibility
Bind 'j'/'k' to moving up/down; add next/previous requests bound to Down/Up
Make -h and --help options ouput a help message
Record builds with dirty working tree by appending -dirty to the version
Make Enter in the pager view always scroll
Add simple window stack, tracking view relation ship via parent member
Move git directory assertion to main; don't require .git repo in pager mode
Add -O2 to CFLAGS to get more warnings
Make update reporting less verbose
Improve title updating and remove flickering
Introduce struct line and use it for view->line
Add support for loading repo config
Make UTF-8 handling optional but still default
Never close backgrounded loads; only clear window when starting to update
Make window switching smother; fix blurring of previous view when switching
When updating the title window, move the cursor to the end of line
Tab size short option changes from -t to -b
Notify that the prompt is unusable while loading
Make the stop all loading request stop all loading
Document the loading time displayed in the title window after 3 seconds
Various usability improvements
Add support for setting color options in ~/.tigrc
Fix segfault where current_view would become >= sizeof(display)
Rename documentation build rules using s/docs/doc/; more like git
Make 'h' and '?' show built-in key binding quick reference
Add support for showing tags and other repo refs in the diff and log view
Move documentation out of tig.c to tig.1.txt, tigrc.5.txt, and manual.txt
Add COPYING, SITES, TODO, BUGS, and INSTALL files
Make the view title show percentage shown like less
Junio C Hamano:
Makefile: make customization of installation locations easier
Kristian Høgsberg:
Bind 'b' to Page Up, and Space to Page Down
Sir Raorn:
Fix linking with --as-needed ld(1) option.
Fix warnings
Timo Hirvonen:
Make some strings "const"
Mark quit() and die() __noreturn
--
Jonas Fonseca
^ permalink raw reply
* Re: New release?
From: Johannes Schindelin @ 2006-06-06 12:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7virnevath.fsf@assigned-by-dhcp.cox.net>
Hi,
On Tue, 6 Jun 2006, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Mon, 5 Jun 2006, Junio C Hamano wrote:
> >
> >> - letting fetch-pack ask for an arbitrary commit object the
> >> user obtained out of band (Eric W Biederman) -- waiting for
> >> updated patch. We would need a corresponding one-liner patch
> >> to upload-pack when we do this.
> >>
> >> This can wait.
> >
> > I think that this could be an important step towards a sensible "shallow
> > clone"...
>
> I did not say we are not interested in doing this ever.
I was not trying to make a case to wait with 1.4.0...
> Also, what you talk about the "lazy clone" is a lot more involved than
> what Eric wanted to have.
A little more involved. And I wanted to know what people think about this
way to tackle shallow clones.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] git-format-patch: add --output-directory long option again
From: Dennis Stosberg @ 2006-06-06 14:19 UTC (permalink / raw)
To: git
Additionally this fixes two minor issues:
(1) git-format-patch left an empty directory behind if an invalid
option was given on the command line.
(2) mkdir() was called with a NULL argument (argv[argc]), if -o was
the last option on the command line.
Signed-off-by: Dennis Stosberg <dennis@stosberg.net>
---
builtin-log.c | 18 +++++++++++-------
1 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index 6612f4c..0018d13 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -206,14 +206,12 @@ int cmd_format_patch(int argc, const cha
keep_subject = 1;
rev.total = -1;
}
- else if (!strcmp(argv[i], "-o")) {
- if (argc < 3)
- die ("Which directory?");
- if (mkdir(argv[i + 1], 0777) < 0 && errno != EEXIST)
- die("Could not create directory %s",
- argv[i + 1]);
- output_directory = strdup(argv[i + 1]);
+ else if (!strcmp(argv[i], "-o") ||
+ !strcmp(argv[i], "--output-directory")) {
i++;
+ if (i == argc)
+ die ("Which directory?");
+ output_directory = strdup(argv[i]);
}
else if (!strcmp(argv[i], "--signoff") ||
!strcmp(argv[i], "-s")) {
@@ -243,6 +241,12 @@ int cmd_format_patch(int argc, const cha
if (argc > 1)
die ("unrecognized argument: %s", argv[1]);
+ if (output_directory && !stdout) {
+ if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
+ die("Could not create directory %s",
+ output_directory);
+ }
+
if (rev.pending_objects && rev.pending_objects->next == NULL) {
rev.pending_objects->item->flags |= UNINTERESTING;
add_head(&rev);
--
1.3.3+git20060602-dest0
^ permalink raw reply related
* Re: [PATCH] git-format-patch: add --output-directory long option again
From: Johannes Schindelin @ 2006-06-06 14:53 UTC (permalink / raw)
To: Dennis Stosberg; +Cc: git
In-Reply-To: <20060606141946.G59b8a1fd@leonov.stosberg.net>
Hi,
what idiot wrote that part of the code originally? ;-)
On Tue, 6 Jun 2006, Dennis Stosberg wrote:
> @@ -243,6 +241,12 @@ int cmd_format_patch(int argc, const cha
> if (argc > 1)
> die ("unrecognized argument: %s", argv[1]);
>
> + if (output_directory && !stdout) {
> + if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
> + die("Could not create directory %s",
> + output_directory);
> + }
> +
> if (rev.pending_objects && rev.pending_objects->next == NULL) {
> rev.pending_objects->item->flags |= UNINTERESTING;
> add_head(&rev);
Would it not be better to
if (output_directory && stdout)
die("What do you want: stdout or a directory?");
Ciao,
Dscho
^ permalink raw reply
* Re: Importing Mozilla CVS into git
From: Jon Smirl @ 2006-06-06 15:13 UTC (permalink / raw)
To: Martin Langhoff; +Cc: git
In-Reply-To: <46a038f90606052255s62cda81bt62d7442beb26658a@mail.gmail.com>
On 6/6/06, Martin Langhoff <martin.langhoff@gmail.com> wrote:
> On 6/3/06, Jon Smirl <jonsmirl@gmail.com> wrote:
> > On 6/1/06, Jon Smirl <jonsmirl@gmail.com> wrote:
> > > With the attached patch you can parse the entire Mozilla tree. The
> > > tree has over 100,000 files in it and about 300 branches.
> >
> > I was a little low with these counts, more like 110,000 files and some
> > parts of the tree have 1,000 branches. Total tree size is 3GB.
>
> I don't think it really has that many branches. If I am to believe
> cvsps (which took 3GB to walk the history), it has some branches with
> recursive loops in their ancestry (MANG_MATH_BRANCH and
> SpiderMonkey140_BRANCH have eachother as ancestors!?), 197969 commits
> and 796 branches.
It probably is 796 and not a 1,000. The branch names were scrolling
across my screen and I just estimated.
> This repository has been mangled quite badly. Don't know what you guys
> did with it, but it sure isn't pretty. I'm working on getting
> git-cvsimport to get through a complete import.
The repository is close to 10 years old and it has gone through a
number of corporate reorgs. Who knows what has happened to it over
that length of time.
Have you looked at the SVN CVS import tool? It imported Mozilla on the
first try. If you download the source they have built about 40 test
repositories with various errors. Those would make a good test suite
for cvsps. http://cvs2svn.tigris.org
I have been working on converting the svn tool to do git commands but
my git knowledge is limited so it has been slow going. The last stage,
pass 8, is very similar to what the git tools do. The svn commands
just need to be swapped for git ones.
If you get git-cvsimport working I'll use it instead. Will the cvsps
process stay small enough to run on a 32b machine? The svn tools are
very RAM efficient since they use an external db. Can cvsps read from
a local copy of the repository without using a CVS server?
We are going to have to develop some kind of incremental mechanism for
updating the new git tree. It can take up to two days to convert the
repository, Mozilla development can't be shut down that long for a
transition. Git will also need to mirror the CVS repository (check-in
still going to CVS) for a long time while we convince everyone on the
merits of switching.
My imported svn version of Mozilla has a lot of performance problems.
One of the directories has over 200,000 files in it slowing downing
the filesystem. The repository went from 3GB CVS to 8GB svn, probably
due to svn using 1000s of tiny files. I'll look around and see if svn
has a pack feature like git.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: New release?
From: Junio C Hamano @ 2006-06-06 15:20 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0606061439500.28953@wbgn013.biozentrum.uni-wuerzburg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> Also, what you talk about the "lazy clone" is a lot more involved than
>> what Eric wanted to have.
>
> A little more involved. And I wanted to know what people think about this
> way to tackle shallow clones.
This truly is a lot more involved, not because "lazy clone" is
hard (which it is) but because what Eric wants to have is not to
produce a shallow clone situation. The issue is about being
able to fetch an object that is not listed as one of the refs
the server side has, and it still will result in a fully
connected repository.
^ permalink raw reply
* Re: [PATCH] Cleanup git-send-email.perl:extract_valid_email
From: Horst von Brand @ 2006-06-06 15:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Horst von Brand, Ryan Anderson, Eric Wong, git
In-Reply-To: <7v1wu2zs5n.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> Horst von Brand <vonbrand@inf.utfsm.cl> writes:
>
> >> > # check for a local address:
> >> > - return $address if ($address =~ /^([\w\-]+)$/);
> >> > + return $address if ($address =~ /^([\w\-.]+)$/);
> >>
> >> I keep forgetting this, '+' is a valid (and useful) setup, too.
> >
> > Oops...
> >>
> >> Actually, I'm retracting my earlier ack on this. This is way too
> >> restrictive. I'd rather allow an occasional invalid email address than
> >> to reject valid ones. I generally trust git users to know what they're
> >> doing when entering email addresses[1].
> >>
> >> *, $, ^, +, = are all valid characters in the username portion (not sure
> >> about local accounts, though), and I'm sure there are more that I don't
> >> know about.
> >
> > As a general principle, I prefer to check what is legal instead of trying
> > to filter out what isn't.
> If we start doing addr-spec in RFC2822 (page 17) ourselves, we
> should rather be using Email::Valid. A permissive sanity check
> to catch obvious mistakes would be more appropriate here than
> being RFC police.
OK.
> I think something like the attached, on top of your patch, would
> be appropriate for upcoming 1.4.0.
>
> -- >8 --
> send-email: be more lenient and just catch obvious mistakes.
>
> This cleans up the pattern matching subroutine by introducing
> two variables to hold regexp to approximately match local-part
> and domain in the e-mail address. It is meant to catch obvious
> mistakes with a cheap check.
>
> The patch also moves "scalar" to force Email::Valid->address()
> to work in !wantarray environment to extract_valid_address;
> earlier it was in the caller of the subroutine, which was way
> too error prone.
Right.
> ---
> diff --git a/git-send-email.perl b/git-send-email.perl
> index a7a7797..700d0c3 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -312,16 +312,18 @@ our ($message_id, $cc, %mail, $subject,
>
> sub extract_valid_address {
> my $address = shift;
> + my $local_part_regexp = '[^<>"\s@]+';
> + my $domain_regexp = '[^.<>"\s@]+\.[^<>"\s@]+';
This forces a '.' in the domain, while vonbrand@localhost is perfectly
reasonable. Plus it doesn't disallow adyacent '.'s. What about:
my $domain_regexp = '[^.<>"\s@]+(\.[^<>"\s@]+)*';
(but this is probably nitpicking...)
--
Dr. Horst H. von Brand User #22616 counter.li.org
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria +56 32 654239
Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513
^ permalink raw reply
* Re: [PATCH] git-format-patch: add --output-directory long option again
From: Junio C Hamano @ 2006-06-06 15:46 UTC (permalink / raw)
To: Dennis Stosberg, Johannes Schindelin; +Cc: git
In-Reply-To: <20060606141946.G59b8a1fd@leonov.stosberg.net>
Dennis Stosberg <dennis@stosberg.net> writes:
> Additionally this fixes two minor issues:
Thanks.
> - else if (!strcmp(argv[i], "-o")) {
> - if (argc < 3)
> - die ("Which directory?");
Oops.
> + if (output_directory && !stdout) {
> + if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
> + die("Could not create directory %s",
> + output_directory);
> + }
> +
This code couldn't have been tested -- you meant to say
"use_stdout" here, not "stdout".
How about this instead?
-- >8 --
[PATCH] git-format-patch: add --output-directory long option again
Additionally notices and complains to an -o option without
directory or a duplicated -o option, -o and --stdout given
together. Also delays the creation of directory until all
arguments are parsed, so that the command does not leave an
empty directory behind when it exits after seeing an unrelated
invalid option.
[Originally from Dennis Stosberg but with minor fixes.]
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
builtin-log.c | 26 ++++++++++++++++----------
1 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index 6612f4c..29a8851 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -103,7 +103,7 @@ static int git_format_config(const char
static FILE *realstdout = NULL;
-static char *output_directory = NULL;
+static const char *output_directory = NULL;
static void reopen_stdout(struct commit *commit, int nr, int keep_subject)
{
@@ -206,14 +206,14 @@ int cmd_format_patch(int argc, const cha
keep_subject = 1;
rev.total = -1;
}
- else if (!strcmp(argv[i], "-o")) {
- if (argc < 3)
- die ("Which directory?");
- if (mkdir(argv[i + 1], 0777) < 0 && errno != EEXIST)
- die("Could not create directory %s",
- argv[i + 1]);
- output_directory = strdup(argv[i + 1]);
+ else if (!strcmp(argv[i], "--output-directory") ||
+ !strcmp(argv[i], "-o")) {
i++;
+ if (argc <= i)
+ die("Which directory?");
+ if (output_directory)
+ die("Two output directories?");
+ output_directory = argv[i];
}
else if (!strcmp(argv[i], "--signoff") ||
!strcmp(argv[i], "-s")) {
@@ -243,6 +243,14 @@ int cmd_format_patch(int argc, const cha
if (argc > 1)
die ("unrecognized argument: %s", argv[1]);
+ if (output_directory) {
+ if (use_stdout)
+ die("standard output, or directory, which one?");
+ if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
+ die("Could not create directory %s",
+ output_directory);
+ }
+
if (rev.pending_objects && rev.pending_objects->next == NULL) {
rev.pending_objects->item->flags |= UNINTERESTING;
add_head(&rev);
@@ -293,8 +301,6 @@ int cmd_format_patch(int argc, const cha
if (!use_stdout)
fclose(stdout);
}
- if (output_directory)
- free(output_directory);
free(list);
return 0;
}
--
1.4.0.rc1-dirty
^ permalink raw reply related
* Re: [PATCH] Cleanup git-send-email.perl:extract_valid_email
From: Junio C Hamano @ 2006-06-06 15:54 UTC (permalink / raw)
To: Horst von Brand; +Cc: git
In-Reply-To: <200606061542.k56Fg9Cm006226@laptop11.inf.utfsm.cl>
Horst von Brand <vonbrand@inf.utfsm.cl> writes:
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index a7a7797..700d0c3 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -312,16 +312,18 @@ our ($message_id, $cc, %mail, $subject,
>>
>> sub extract_valid_address {
>> my $address = shift;
>> + my $local_part_regexp = '[^<>"\s@]+';
>> + my $domain_regexp = '[^.<>"\s@]+\.[^<>"\s@]+';
>
> This forces a '.' in the domain, while vonbrand@localhost is perfectly
> reasonable. Plus it doesn't disallow adyacent '.'s. What about:
>
> my $domain_regexp = '[^.<>"\s@]+(\.[^<>"\s@]+)*';
>
> (but this is probably nitpicking...)
I do not have preference either way about allowing an address
like tld-administrator@net myself, but Email::Valid->address
does not seem to allow it, and I just copied that behaviour for
consistency between two alternative implementations.
I think you meant to say:
> my $domain_regexp = '[^.<>"\s@]+(\.[^.<>"\s@]+)*';
(i.e. exclude dot from the latter character class), but I am
inclined to do this instead:
my $domain_regexp = '[^.<>"\s@]+(?:\.[^.<>"\s@]+)+';
(i.e. still require at least two levels).
^ permalink raw reply
* Re: [PATCH] Cleanup git-send-email.perl:extract_valid_email
From: Horst von Brand @ 2006-06-06 16:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Horst von Brand, git
In-Reply-To: <7vpshmth3q.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> Horst von Brand <vonbrand@inf.utfsm.cl> writes:
>
> >> diff --git a/git-send-email.perl b/git-send-email.perl
> >> index a7a7797..700d0c3 100755
> >> --- a/git-send-email.perl
> >> +++ b/git-send-email.perl
> >> @@ -312,16 +312,18 @@ our ($message_id, $cc, %mail, $subject,
> >>
> >> sub extract_valid_address {
> >> my $address = shift;
> >> + my $local_part_regexp = '[^<>"\s@]+';
> >> + my $domain_regexp = '[^.<>"\s@]+\.[^<>"\s@]+';
> >
> > This forces a '.' in the domain, while vonbrand@localhost is perfectly
> > reasonable. Plus it doesn't disallow adyacent '.'s. What about:
> >
> > my $domain_regexp = '[^.<>"\s@]+(\.[^<>"\s@]+)*';
> >
> > (but this is probably nitpicking...)
>
> I do not have preference either way about allowing an address
> like tld-administrator@net myself, but Email::Valid->address
> does not seem to allow it, and I just copied that behaviour for
> consistency between two alternative implementations.
Reasonable.
> I think you meant to say:
>
> > my $domain_regexp = '[^.<>"\s@]+(\.[^.<>"\s@]+)*';
>
> (i.e. exclude dot from the latter character class),
Right, my bad.
> but I am
> inclined to do this instead:
>
> my $domain_regexp = '[^.<>"\s@]+(?:\.[^.<>"\s@]+)+';
>
> (i.e. still require at least two levels).
OK, but be careful as this (?:...) is an extended regexp (needs /x on
match). I'd just leave it plain (the performance impact shouldn't be
noticeable). I don't see any use except for $1, so the extra parenthesis
should be safe.
--
Dr. Horst H. von Brand User #22616 counter.li.org
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria +56 32 654239
Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513
^ permalink raw reply
* Re: New release?
From: Jon Loeliger @ 2006-06-06 16:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List, Linus Torvalds
In-Reply-To: <7vodx6zus2.fsf@assigned-by-dhcp.cox.net>
On Tue, 2006-06-06 at 01:02, Junio C Hamano wrote:
> The "next" queue has been shrinking, and nothing is going to
> "maint" branch, which can mean only one thing ;-).
Junio,
If possible, I'd like to consider my (Linus') notion
of the extension to the git protocol supplying the client
hostname as part of a "standard" release like 1.4.0.
We don't necessarily have to agree on how it is used yet,
but if we can get the protocol enhancement into this
release, I think it would make for a good "flag day"
sort of change.
Specifically, I'd like to consider the portion my patch
that passes "\0HOST=%s\0" along with the git: protocol
connection.
Any chance for that? If you'd like, I work on resubmitting
just those bits with the connect function refactoring
that you outlined.
Thanks,
jdl
^ permalink raw reply
* Re: New release?
From: Pavel Roskin @ 2006-06-06 16:19 UTC (permalink / raw)
To: git, Junio C Hamano; +Cc: Linus Torvalds
In-Reply-To: <7vodx6zus2.fsf@assigned-by-dhcp.cox.net>
Hello, Junio!
On Mon, 2006-06-05 at 23:02 -0700, Junio C Hamano wrote:
> - http-fetch fixes from Nick, which looked obviously correct.
> I would appreciate test reports from people who saw breakages
> on this one.
I'm still getting a segfault with the current git from the "next" branch:
$ git-clone http://www.denx.de/git/linux-2.6-denx.git
...
got 4160b8334c53e0881cdc12c1f7d3d54fff883772
got 5f57f29efee48d84e235a8ff75a35e7e354227a7
got 681a9c73a2a321850404d4856f4738be47e17d15
got 29b0ddaa324e417abf153460d7d94fb67823a6ef
got 23e7a5c7d2c13d98524b69f54378d887e1962fc8
/home/proski/bin/git-clone: line 29: 27271 Segmentation fault git-http-fetch -v -a -w "$tname" "$name" "$1/"
It takes about an hour with my connection from running the command to
the segfault. It you have any idea how to speed it up, it would be very
helpful.
If I comment out USE_CURL_MULTI, I get compile errors in http.c and
http-push.c.
http.c: In function 'release_active_slot':
http.c:441: error: 'curlm' undeclared (first use in this function)
http.c:441: error: (Each undeclared identifier is reported only once
http.c:441: error: for each function it appears in.)
http.c:445: warning: implicit declaration of function
'fill_active_slots'
http-push.c: In function 'fill_active_slots':
http-push.c:801: error: 'max_requests' undeclared (first use in this
function)
http-push.c:801: error: (Each undeclared identifier is reported only
once
http-push.c:801: error: for each function it appears in.)
http-push.c:811: error: 'curlm' undeclared (first use in this function)
http-push.c: In function 'add_fetch_request':
http-push.c:855: warning: implicit declaration of function
'step_active_slots'
It may be easy to fix for somebody familiar with the code, but I don't
want to do it blindly.
--
Regards,
Pavel Roskin
^ permalink raw reply
* [PATCH] HTTP cleanup
From: Nick Hengeveld @ 2006-06-06 16:41 UTC (permalink / raw)
To: git
Fix broken build when USE_CURL_MULTI is not defined, as noted by Becky Bruce.
During cleanup, free header slist that was created during init, as noted
by Junio.
Signed-off-by: Nick Hengeveld <nickh@reactrix.com>
---
I've verified that git will build with USE_CURL_MULTI commented out, but
have done very little testing of the resulting fetch/push binaries.
http-push.c | 8 ++++++++
http.c | 5 +++++
2 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/http-push.c b/http-push.c
index b1c018a..40524a8 100644
--- a/http-push.c
+++ b/http-push.c
@@ -788,6 +788,7 @@ static void finish_request(struct transf
}
}
+#ifdef USE_CURL_MULTI
void fill_active_slots(void)
{
struct transfer_request *request = request_queue_head;
@@ -821,6 +822,7 @@ void fill_active_slots(void)
slot = slot->next;
}
}
+#endif
static void get_remote_object_list(unsigned char parent);
@@ -851,8 +853,10 @@ static void add_fetch_request(struct obj
request->next = request_queue_head;
request_queue_head = request;
+#ifdef USE_CURL_MULTI
fill_active_slots();
step_active_slots();
+#endif
}
static int add_send_request(struct object *obj, struct remote_lock *lock)
@@ -889,8 +893,10 @@ static int add_send_request(struct objec
request->next = request_queue_head;
request_queue_head = request;
+#ifdef USE_CURL_MULTI
fill_active_slots();
step_active_slots();
+#endif
return 1;
}
@@ -2523,7 +2529,9 @@ int main(int argc, char **argv)
if (objects_to_send)
fprintf(stderr, " sending %d objects\n",
objects_to_send);
+#ifdef USE_CURL_MULTI
fill_active_slots();
+#endif
finish_all_active_slots();
/* Update the remote branch if all went well */
diff --git a/http.c b/http.c
index 146cf7b..a2e8e78 100644
--- a/http.c
+++ b/http.c
@@ -287,6 +287,7 @@ #ifdef USE_CURL_MULTI
#endif
curl_global_cleanup();
+ curl_slist_free_all(pragma_header);
}
struct active_request_slot *get_active_slot(void)
@@ -438,11 +439,15 @@ void release_active_slot(struct active_r
{
closedown_active_slot(slot);
if (slot->curl) {
+#ifdef USE_CURL_MULTI
curl_multi_remove_handle(curlm, slot->curl);
+#endif
curl_easy_cleanup(slot->curl);
slot->curl = NULL;
}
+#ifdef USE_CURL_MULTI
fill_active_slots();
+#endif
}
static void finish_active_slot(struct active_request_slot *slot)
--
1.3.3.g423a-dirty
^ permalink raw reply related
* Re: New release?
From: Nick Hengeveld @ 2006-06-06 16:46 UTC (permalink / raw)
To: Pavel Roskin; +Cc: git, Junio C Hamano, Linus Torvalds
In-Reply-To: <1149610759.27253.9.camel@dv>
On Tue, Jun 06, 2006 at 12:19:19PM -0400, Pavel Roskin wrote:
> On Mon, 2006-06-05 at 23:02 -0700, Junio C Hamano wrote:
> > - http-fetch fixes from Nick, which looked obviously correct.
> > I would appreciate test reports from people who saw breakages
> > on this one.
>
> I'm still getting a segfault with the current git from the "next" branch:
>
> $ git-clone http://www.denx.de/git/linux-2.6-denx.git
> ...
> got 4160b8334c53e0881cdc12c1f7d3d54fff883772
> got 5f57f29efee48d84e235a8ff75a35e7e354227a7
> got 681a9c73a2a321850404d4856f4738be47e17d15
> got 29b0ddaa324e417abf153460d7d94fb67823a6ef
> got 23e7a5c7d2c13d98524b69f54378d887e1962fc8
> /home/proski/bin/git-clone: line 29: 27271 Segmentation fault git-http-fetch -v -a -w "$tname" "$name" "$1/"
>
> It takes about an hour with my connection from running the command to
> the segfault. It you have any idea how to speed it up, it would be very
> helpful.
I can't offer any suggestions wrt speeding it up, but I'll do what I can
to reproduce the problem here.
> If I comment out USE_CURL_MULTI, I get compile errors in http.c and
> http-push.c.
I just posted a fix for the compile errors. As noted there, I've done
very little testing of the fetch/push binaries when built with
USE_CURL_MULTI commented out.
--
For a successful technology, reality must take precedence over public
relations, for nature cannot be fooled.
^ permalink raw reply
* Re: [PATCH] Cleanup git-send-email.perl:extract_valid_email
From: Junio C Hamano @ 2006-06-06 16:58 UTC (permalink / raw)
To: Horst von Brand; +Cc: git
In-Reply-To: <200606061605.k56G5gHo006581@laptop11.inf.utfsm.cl>
Horst von Brand <vonbrand@inf.utfsm.cl> writes:
> Junio C Hamano <junkio@cox.net> wrote:
>> but I am
>> inclined to do this instead:
>>
>> my $domain_regexp = '[^.<>"\s@]+(?:\.[^.<>"\s@]+)+';
>>
>> (i.e. still require at least two levels).
>
> OK, but be careful as this (?:...) is an extended regexp (needs /x on
> match).
Are you sure about /x?
^ permalink raw reply
* Re: New release?
From: Junio C Hamano @ 2006-06-06 17:15 UTC (permalink / raw)
To: Jon Loeliger; +Cc: Git List, Linus Torvalds
In-Reply-To: <1149610100.23938.75.camel@cashmere.sps.mot.com>
Jon Loeliger <jdl@freescale.com> writes:
> If possible, I'd like to consider my (Linus') notion
> of the extension to the git protocol supplying the client
> hostname as part of a "standard" release like 1.4.0.
I think the virtual hosting of git-daemon is very important, and
I do not want to have a half-baked one hurriedly cobbled
together for 1.4.0 on the server side.
> We don't necessarily have to agree on how it is used yet,
> but if we can get the protocol enhancement into this
> release, I think it would make for a good "flag day"
> sort of change.
I do agree it is a good idea to have the client-side support in
1.4.0; that would work well with the current git-daemon. Since
the extension is backward compatible, we can tell the users to
use 1.4.0 or later clients when the daemon side is done. That
is much nicer than telling them to use what's on "master" that
is later than such-and-such commit.
> Specifically, I'd like to consider the portion my patch
> that passes "\0HOST=%s\0" along with the git: protocol
> connection.
>
> Any chance for that? If you'd like, I work on resubmitting
> just those bits with the connect function refactoring
> that you outlined.
I appreciate the offer very much. Although I said port is
something the server side can usually tell, I would throw in
port there as well. Maybe there is some port forwarding (or
reverse NAPT) involved in the path from the client to the
server. Any reason to spell HOST in all uppercase by the way?
And thanks for reminding me. The list of things-to-have in the
message was based on a week old "What's in", and I was about to
compose a message describing what I did not talk about in it,
because I wanted to say something about virtual hosting of
git-daemon.
^ permalink raw reply
* Re: New release?
From: Jon Loeliger @ 2006-06-06 17:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List, Linus Torvalds
In-Reply-To: <7v3beitdct.fsf@assigned-by-dhcp.cox.net>
On Tue, 2006-06-06 at 12:15, Junio C Hamano wrote:
> I think the virtual hosting of git-daemon is very important, and
> I do not want to have a half-baked one hurriedly cobbled
> together for 1.4.0 on the server side.
Agreed.
> I do agree it is a good idea to have the client-side support in
> 1.4.0; that would work well with the current git-daemon. Since
> the extension is backward compatible, we can tell the users to
> use 1.4.0 or later clients when the daemon side is done. That
> is much nicer than telling them to use what's on "master" that
> is later than such-and-such commit.
Exactly.
> > Any chance for that? If you'd like, I work on resubmitting
> > just those bits with the connect function refactoring
> > that you outlined.
>
> I appreciate the offer very much.
No problem.
> Although I said port is
> something the server side can usually tell, I would throw in
> port there as well.
Can do.
> Any reason to spell HOST in all uppercase by the way?
I would never suggest that it came to me from
gittus in an IRC day-dream...? :-)
Consider it all lowercaseified instead.
jdl
^ permalink raw reply
* Re: [PATCH] git-format-patch: add --output-directory long option again
From: Dennis Stosberg @ 2006-06-06 18:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vwtbuthgw.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> > + if (output_directory && !stdout) {
> > + if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
> > + die("Could not create directory %s",
> > + output_directory);
> > + }
> > +
>
> This code couldn't have been tested -- you meant to say
> "use_stdout" here, not "stdout".
Sorry. My version compiled and ran cleanly, but I was blinded by my
goal: I wanted the directory to be _not_ created and checked that,
but I didn't make sure, that the directory really existed in the
other case, since Git didn't produce an error.
> How about this instead?
> [...]
> + if (output_directory) {
> + if (use_stdout)
> + die("standard output, or directory, which one?");
Looks good. This piece changes behaviour; it no longer matches the
docs.
Regards,
Dennis
---
Documentation/git-format-patch.txt | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 493cac2..4ca0014 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -40,8 +40,7 @@ OPTIONS
-------
-o|--output-directory <dir>::
Use <dir> to store the resulting files, instead of the
- current working directory. This option is ignored if
- --stdout is specified.
+ current working directory.
-n|--numbered::
Name output in '[PATCH n/m]' format.
^ permalink raw reply related
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