* Re: [PATCHv2] connect: display connection progress
From: Michael S. Tsirkin @ 2007-05-10 12:25 UTC (permalink / raw)
To: Alex Riesen; +Cc: Michael S. Tsirkin, Junio C Hamano, git
In-Reply-To: <81b0412b0705100519i3028fbc4y25e7c407c7c8216@mail.gmail.com>
> Quoting Alex Riesen <raa.lkml@gmail.com>:
> Subject: Re: [PATCHv2] connect: display connection progress
>
> On 5/10/07, Michael S. Tsirkin <mst@dev.mellanox.co.il> wrote:
> >> Quoting Alex Riesen <raa.lkml@gmail.com>:
> >> Subject: Re: [PATCHv2] connect: display connection progress
> >>
> >> On 5/10/07, Michael S. Tsirkin <mst@dev.mellanox.co.il> wrote:
> >> >-static int git_tcp_connect_sock(char *host)
> >> >+static int git_tcp_connect_sock(char *host, int flags)
> >>
> >> There is only one bit of flags ever used. What are the others for?
> >
> >Hmm, I thought it's easier to read
> >git_tcp_connect_sock(host, NET_QUIET)
>
> It is easier to read. "int flags" isn't easier to understand.
>
> >> Why use negative logic?
> >> What was wrong with plain "int verbose"?
> >
> >I want the default to report connections, and -q
> >to silence them. Maybe "int quiet"?
>
> It depends. "Quiet" is negative, which automatically
> makes the logic harder to follow (for humans, at least),
> and you had to put negations all over git_tcp_connect,
> exactly because the meaning is exactly the opposite to
> what you need.
>
> >> What addresses were tried by connect?
> >
> >You are speaking about your patch reporting the IP on failure?
>
> Yes. Not on failure (not only). Every time an address is tried
> to connect.
Why not only on failure? IP addresses look ugly.
> >I think it makes sense, but it's a separate issue, isn't it?
>
> You are just about to make git_tcp_connect verbose,
> are you not?
Only if the flag is set. So git-fetch without -q qill be more verbose -
but it already spits out a fair amount of data on screen.
--
MST
^ permalink raw reply
* Re: [PATCHv2] connect: display connection progress
From: Alex Riesen @ 2007-05-10 12:19 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Junio C Hamano, git
In-Reply-To: <20070510120802.GG13655@mellanox.co.il>
On 5/10/07, Michael S. Tsirkin <mst@dev.mellanox.co.il> wrote:
> > Quoting Alex Riesen <raa.lkml@gmail.com>:
> > Subject: Re: [PATCHv2] connect: display connection progress
> >
> > On 5/10/07, Michael S. Tsirkin <mst@dev.mellanox.co.il> wrote:
> > >-static int git_tcp_connect_sock(char *host)
> > >+static int git_tcp_connect_sock(char *host, int flags)
> >
> > There is only one bit of flags ever used. What are the others for?
>
> Hmm, I thought it's easier to read
> git_tcp_connect_sock(host, NET_QUIET)
It is easier to read. "int flags" isn't easier to understand.
> > Why use negative logic?
> > What was wrong with plain "int verbose"?
>
> I want the default to report connections, and -q
> to silence them. Maybe "int quiet"?
It depends. "Quiet" is negative, which automatically
makes the logic harder to follow (for humans, at least),
and you had to put negations all over git_tcp_connect,
exactly because the meaning is exactly the opposite to
what you need.
> > What addresses were tried by connect?
>
> You are speaking about your patch reporting the IP on failure?
Yes. Not on failure (not only). Every time an address is tried
to connect.
> I think it makes sense, but it's a separate issue, isn't it?
You are just about to make git_tcp_connect verbose,
are you not?
^ permalink raw reply
* Re: [PATCHv2] connect: display connection progress
From: Michael S. Tsirkin @ 2007-05-10 12:08 UTC (permalink / raw)
To: Alex Riesen; +Cc: Michael S. Tsirkin, Junio C Hamano, git
In-Reply-To: <81b0412b0705100439j4e6b072bk1ba19a4f971e5d0c@mail.gmail.com>
> Quoting Alex Riesen <raa.lkml@gmail.com>:
> Subject: Re: [PATCHv2] connect: display connection progress
>
> On 5/10/07, Michael S. Tsirkin <mst@dev.mellanox.co.il> wrote:
> >-static int git_tcp_connect_sock(char *host)
> >+static int git_tcp_connect_sock(char *host, int flags)
>
> There is only one bit of flags ever used. What are the others for?
Hmm, I thought it's easier to read
git_tcp_connect_sock(host, NET_QUIET)
than
git_tcp_connect_sock(host, 1)
but maybe that's overdesign.
> Why use negative logic?
> What was wrong with plain "int verbose"?
I want the default to report connections, and -q
to silence them. Maybe "int quiet"?
> What addresses were tried by connect?
You are speaking about your patch reporting the IP on failure?
I think it makes sense, but it's a separate issue, isn't it?
--
MST
^ permalink raw reply
* Re: Merging commits together into a super-commit
From: Johannes Sixt @ 2007-05-10 11:40 UTC (permalink / raw)
To: git
In-Reply-To: <1178794261.5806.98.camel@murta.transitives.com>
Alex Bennee wrote:
> I really love the fact I can micro-commit changes when I'm developing.
> However at some point the combination of changes I have made can be
> considered a single body of work. This is especially true when you start
> doing things like re-basing on code that has moved around a lot. You
> don't want to be correcting a whole bunch of merge failures for every
> commit in your current tree.
>
> So far the only was I can see to do this is a:
>
> git-diff master..HEAD > my.patch
>
> And then re-applying your patch in stages, manually doing the commits.
>
> Am I missing something?
>
> I'm thinking something like git-cherrypick taking multiple commits and
> create a new super commit on a new tree. i.e.:
>
> git-cherrypick -m "Valgrind fixes" 12345.. 12678.. 565757..
>
> Merging the existing commit comments would be nice too.
Here we go:
- cherry-pick them before commit
$ git cherry-pick -n x
$ git cherry-pick -n y
$ git cherry-pick -n z
$ git commit -m "$(for c in x y z; do git show --stat $c; done)" -e
- merge in a single commit
$ git merge --squash foo
You didn't really think that git couldn't do that, did you? ;)
-- Hannes
^ permalink raw reply
* RE: Merging commits together into a super-commit
From: Johannes Schindelin @ 2007-05-10 11:43 UTC (permalink / raw)
To: Alex Bennee; +Cc: Raimund Bauer, git
In-Reply-To: <1178796748.5806.102.camel@murta.transitives.com>
Hi,
On Thu, 10 May 2007, Alex Bennee wrote:
> On Thu, 2007-05-10 at 13:19 +0200, Raimund Bauer wrote:
> > > Hi,
> > <snip>
> > You don't want to be
> > > correcting a whole bunch of merge failures for every commit
> > > in your current tree.
> > >
> > > So far the only was I can see to do this is a:
> > >
> > > git-diff master..HEAD > my.patch
> > >
> > > And then re-applying your patch in stages, manually doing the commits.
> > >
> > > Am I missing something?
> >
> > git merge --squash ?
>
> Hmm, that would do although it only works on whole trees, you can't
> specify a range of commits. So you would have to cherrypick groups of
> changes onto other branches and then merge them together with a couple
> of --squashes
Since specifying several commit ranges can always lead to merge conflicts,
it makes no sense to _not_ accumulate the patches in a separate branch.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCHv2] connect: display connection progress
From: Alex Riesen @ 2007-05-10 11:39 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Junio C Hamano, git
In-Reply-To: <20070510095156.GC13655@mellanox.co.il>
On 5/10/07, Michael S. Tsirkin <mst@dev.mellanox.co.il> wrote:
> -static int git_tcp_connect_sock(char *host)
> +static int git_tcp_connect_sock(char *host, int flags)
There is only one bit of flags ever used. What are the others for?
Why use negative logic?
What was wrong with plain "int verbose"?
What addresses were tried by connect?
^ permalink raw reply
* RE: Merging commits together into a super-commit
From: Alex Bennee @ 2007-05-10 11:32 UTC (permalink / raw)
To: Raimund Bauer; +Cc: git
In-Reply-To: <000e01c792f5$0861abd0$0b0aa8c0@abf.local>
On Thu, 2007-05-10 at 13:19 +0200, Raimund Bauer wrote:
> > Hi,
> <snip>
> You don't want to be
> > correcting a whole bunch of merge failures for every commit
> > in your current tree.
> >
> > So far the only was I can see to do this is a:
> >
> > git-diff master..HEAD > my.patch
> >
> > And then re-applying your patch in stages, manually doing the commits.
> >
> > Am I missing something?
>
> git merge --squash ?
Hmm, that would do although it only works on whole trees, you can't
specify a range of commits. So you would have to cherrypick groups of
changes onto other branches and then merge them together with a couple
of --squashes
However thanks for pointing that one out :-)
--
Alex, homepage: http://www.bennee.com/~alex/
A pretty woman can do anything; an ugly woman must do everything.
^ permalink raw reply
* Re: FFmpeg considering GIT
From: Marco Costalba @ 2007-05-10 11:20 UTC (permalink / raw)
To: Jan Hudec
Cc: Fredrik Kuivinen, Paul Mackerras, Alex Riesen, Linus Torvalds,
Karl Hasselstr?m, Junio C Hamano, Carl Worth, Michael Niedermayer,
Git Mailing List
In-Reply-To: <20070509213610.GA9144@efreet.light.src>
On 5/9/07, Jan Hudec <bulb@ucw.cz> wrote:
> On Wed, May 09, 2007 at 23:09:25 +0200, Fredrik Kuivinen wrote:
> > I have used PyQt for some smaller projects (notably Hgct, a no longer
> > developed
> > commit tool for git and Mercurial. See
> > http://repo.or.cz/w/hgct.git?a=tree). For me
> > PyQt has worked very well. The python interface to Qt is more or less a
> > direct
> > translation of the C++ interface, so the excellent documentation troll
> > tech provides
> > for Qt can be used when developing with PyQt as well.
> >
> > I have never seen the segfaulting you mention. Maybe my programs have been
> > too
> > small to trigger that bug...
>
> It's not about size of the programs. It's about having to be careful not to
> refer to widgets inside eg. dialog box from outside and close that dialog
> box.
In Qt all the classes that ineriths from QObject are memory managed,
to be more clear
you can say that one class is "child" of another class (always
ineritherd from QObject) that becames the parent.
When you delete the parent, all his children are deleted too, this is
a (big) feature to avoid
missing free() calls for resources created with mallocs() , (well, in
C++ we say 'delete' for resources created by 'new' but the concept is
more or less the same).
Note that this property can be nested: create a main window, inside a
window there is a tab form, inside the tab there is a list view,
inside the list view there are items (lines of list view).
So *when* you delete the main window all this stuff is automatically
deleted by Qt. It is diffrent from a garbage collector because there
is no delay in releasing memory and all the thing is strict
deterministic.
So coming to your problem, if you need to refer to a widget inside a
dialog *after* the dialog has been deleted you can simply reparent to
NULL the widget before closing the dialog so to remove your object
from the delete list of the dialog.
Another option, in case your obect is not a graphical widget, is to
avoid declaring your object "child" of the dialog in first instance
setting his parent to NULL. This is clearly better because documents
'in code' also the real relationship between the dialog and your
object.
Marco
^ permalink raw reply
* RE: Merging commits together into a super-commit
From: Raimund Bauer @ 2007-05-10 11:19 UTC (permalink / raw)
To: 'Alex Bennee', git
In-Reply-To: <1178794261.5806.98.camel@murta.transitives.com>
> Hi,
>
> I really love the fact I can micro-commit changes when I'm
> developing. However at some point the combination of changes
> I have made can be considered a single body of work. This is
> especially true when you start doing things like re-basing on
> code that has moved around a lot. You don't want to be
> correcting a whole bunch of merge failures for every commit
> in your current tree.
>
> So far the only was I can see to do this is a:
>
> git-diff master..HEAD > my.patch
>
> And then re-applying your patch in stages, manually doing the commits.
>
> Am I missing something?
git merge --squash ?
--
best regards
Ray
^ permalink raw reply
* Merging commits together into a super-commit
From: Alex Bennee @ 2007-05-10 10:51 UTC (permalink / raw)
To: git
Hi,
I really love the fact I can micro-commit changes when I'm developing.
However at some point the combination of changes I have made can be
considered a single body of work. This is especially true when you start
doing things like re-basing on code that has moved around a lot. You
don't want to be correcting a whole bunch of merge failures for every
commit in your current tree.
So far the only was I can see to do this is a:
git-diff master..HEAD > my.patch
And then re-applying your patch in stages, manually doing the commits.
Am I missing something?
I'm thinking something like git-cherrypick taking multiple commits and
create a new super commit on a new tree. i.e.:
git-cherrypick -m "Valgrind fixes" 12345.. 12678.. 565757..
Merging the existing commit comments would be nice too.
--
Alex, homepage: http://www.bennee.com/~alex/
All God's children are not beautiful. Most of God's children are, in
fact, barely presentable. -- Fran Lebowitz, "Metropolitan Life"
^ permalink raw reply
* [PATCH] t5000: skip ZIP tets when unzip is absent
From: Johannes Schindelin @ 2007-05-10 10:47 UTC (permalink / raw)
To: junkio, git
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
t/t5000-tar-tree.sh | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index e223c07..1580592 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -108,6 +108,13 @@ test_expect_success \
'git-archive --format=zip' \
'git-archive --format=zip HEAD >d.zip'
+unzip -v 2>/dev/null
+if [ $? -ne 10 ]; then
+ echo "Skipping ZIP tests, because unzip was not found"
+ test_done
+ exit
+fi
+
test_expect_success \
'extract ZIP archive' \
'(mkdir d && cd d && $UNZIP ../d.zip)'
^ permalink raw reply related
* [PATCH] Add a birdview-on-the-source-code section to the user manual
From: Johannes Schindelin @ 2007-05-10 10:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: J. Bruce Fields, kha, barkalow, git
In-Reply-To: <7vk5vhnwl1.fsf@assigned-by-dhcp.cox.net>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 11208 bytes --]
In http://thread.gmane.org/gmane.comp.version-control.git/42479,
a birdview on the source code was requested.
J. Bruce Fields suggested that my reply should be included in the
user manual, and there was nothing of an outcry, so here it is,
not 2 months later.
It includes modifications as suggested by J. Bruce Fields, Karl
Hasselström and Daniel Barkalow.
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
On Wed, 9 May 2007, Junio C Hamano wrote:
> "J. Bruce Fields" <bfields@fieldses.org> writes:
>
> > On Wed, May 09, 2007 at 02:19:03PM +0200, Johannes Schindelin wrote:
> >> [PATCH] user-manual: Touch ups on the birdview section
> >
> > Those all look like sensible changes to me, thanks!
>
> Likewise, except that big-endian bit I think everybody agrees on
> just dropping.
And here it is, in its full glory, prepared in a way which
appeals to the maintainers, hopefully...
Documentation/user-manual.txt | 219 +++++++++++++++++++++++++++++++++++++++++
1 files changed, 219 insertions(+), 0 deletions(-)
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 13db969..3c3f1b4 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -3160,6 +3160,225 @@ confusing and scary messages, but it won't actually do anything bad. In
contrast, running "git prune" while somebody is actively changing the
repository is a *BAD* idea).
+[[birdview-on-the-source-code]]
+A birdview on Git's source code
+-----------------------------
+
+While Git's source code is quite elegant, it is not always easy for
+new developers to find their way through it. A good idea is to look
+at the contents of the initial commit:
+_e83c5163316f89bfbde7d9ab23ca2e25604af290_ (also known as _v0.99~954_).
+
+Tip: you can see what files are in there with
+
+----------------------------------------------------
+$ git show e83c5163316f89bfbde7d9ab23ca2e25604af290:
+----------------------------------------------------
+
+and look at those files with something like
+
+-----------------------------------------------------------
+$ git show e83c5163316f89bfbde7d9ab23ca2e25604af290:cache.h
+-----------------------------------------------------------
+
+Be sure to read the README in that revision _after_ you are familiar with
+the terminology (<<glossary>>), since the terminology has changed a little
+since then. For example, we call the things "commits" now, which are
+described in that README as "changesets".
+
+Actually a lot of the structure as it is now can be explained by that
+initial commit.
+
+For example, we do not call it "cache" any more, but "index", however, the
+file is still called `cache.h`. Remark: Not much reason to change it now,
+especially since there is no good single name for it anyway, because it is
+basically _the_ header file which is included by _all_ of Git's C sources.
+
+If you grasp the ideas in that initial commit (it is really small and you
+can get into it really fast, and it will help you recognize things in the
+much larger code base we have now), you should go on skimming `cache.h`,
+`object.h` and `commit.h` in the current version.
+
+In the early days, Git (in the tradition of UNIX) was a bunch of programs
+which were extremely simple, and which you used in scripts, piping the
+output of one into another. This turned out to be good for initial
+development, since it was easier to test new things. However, recently
+many of these parts have become builtins, and some of the core has been
+"libified", i.e. put into libgit.a for performance, portability reasons,
+and to avoid code duplication.
+
+By now, you know what the index is (and find the corresponding data
+structures in `cache.h`), and that there are just a couple of object types
+(blobs, trees, commits and tags) which inherit their common structure from
+`struct object`, which is their first member (and thus, you can cast e.g.
+`(struct object *)commit` to achieve the _same_ as `&commit->object`, i.e.
+get at the object name and flags).
+
+Now is a good point to take a break to let this information sink in.
+
+Next step: get familiar with the object naming. Read <<naming-commits>>.
+There are quite a few ways to name an object (and not only revisions!).
+All of these are handled in `sha1_name.c`. Just have a quick look at
+the function `get_sha1()`. A lot of the special handling is done by
+functions like `get_sha1_basic()` or the likes.
+
+This is just to get you into the groove for the most libified part of Git:
+the revision walker.
+
+Basically, the initial version of `git log` was a shell script:
+
+----------------------------------------------------------------
+$ git-rev-list --pretty $(git-rev-parse --default HEAD "$@") | \
+ LESS=-S ${PAGER:-less}
+----------------------------------------------------------------
+
+What does this mean?
+
+`git-rev-list` is the original version of the revision walker, which
+_always_ printed a list of revisions to stdout. It is still functional,
+and needs to, since most new Git programs start out as scripts using
+`git-rev-list`.
+
+`git-rev-parse` is not as important any more; it was only used to filter out
+options that were relevant for the different plumbing commands that were
+called by the script.
+
+Most of what `git-rev-list` did is contained in `revision.c` and
+`revision.h`. It wraps the options in a struct named `rev_info`, which
+controls how and what revisions are walked, and more.
+
+The original job of `git-rev-parse` is now taken by the function
+`setup_revisions()`, which parses the revisions and the common command line
+options for the revision walker. This information is stored in the struct
+`rev_info` for later consumption. You can do your own command line option
+parsing after calling `setup_revisions()`. After that, you have to call
+`prepare_revision_walk()` for initialization, and then you can get the
+commits one by one with the function `get_revision()`.
+
+If you are interested in more details of the revision walking process,
+just have a look at the first implementation of `cmd_log()`; call
+`git-show v1.3.0~155^2~4` and scroll down to that function (note that you
+no longer need to call `setup_pager()` directly).
+
+Nowadays, `git log` is a builtin, which means that it is _contained_ in the
+command `git`. The source side of a builtin is
+
+- a function called `cmd_<bla>`, typically defined in `builtin-<bla>.c`,
+ and declared in `builtin.h`,
+
+- an entry in the `commands[]` array in `git.c`, and
+
+- an entry in `BUILTIN_OBJECTS` in the `Makefile`.
+
+Sometimes, more than one builtin is contained in one source file. For
+example, `cmd_whatchanged()` and `cmd_log()` both reside in `builtin-log.c`,
+since they share quite a bit of code. In that case, the commands which are
+_not_ named like the `.c` file in which they live have to be listed in
+`BUILT_INS` in the `Makefile`.
+
+`git log` looks more complicated in C than it does in the original script,
+but that allows for a much greater flexibility and performance.
+
+Here again it is a good point to take a pause.
+
+Lesson three is: study the code. Really, it is the best way to learn about
+the organization of Git (after you know the basic concepts).
+
+So, think about something which you are interested in, say, "how can I
+access a blob just knowing the object name of it?". The first step is to
+find a Git command with which you can do it. In this example, it is either
+`git show` or `git cat-file`.
+
+For the sake of clarity, let's stay with `git cat-file`, because it
+
+- is plumbing, and
+
+- was around even in the initial commit (it literally went only through
+ some 20 revisions as `cat-file.c`, was renamed to `builtin-cat-file.c`
+ when made a builtin, and then saw less than 10 versions).
+
+So, look into `builtin-cat-file.c`, search for `cmd_cat_file()` and look what
+it does.
+
+------------------------------------------------------------------
+ git_config(git_default_config);
+ if (argc != 3)
+ usage("git-cat-file [-t|-s|-e|-p|<type>] <sha1>");
+ if (get_sha1(argv[2], sha1))
+ die("Not a valid object name %s", argv[2]);
+------------------------------------------------------------------
+
+Let's skip over the obvious details; the only really interesting part
+here is the call to `get_sha1()`. It tries to interpret `argv[2]` as an
+object name, and if it refers to an object which is present in the current
+repository, it writes the resulting SHA-1 into the variable `sha1`.
+
+Two things are interesting here:
+
+- `get_sha1()` returns 0 on _success_. This might surprise some new
+ Git hackers, but there is a long tradition in UNIX to return different
+ negative numbers in case of different errors -- and 0 on success.
+
+- the variable `sha1` in the function signature of `get_sha1()` is `unsigned
+ char *`, but is actually expected to be a pointer to `unsigned
+ char[20]`. This variable will contain the 160-bit SHA-1 of the given
+ commit. Note that whenever a SHA-1 is passed as "unsigned char *", it
+ is the binary representation, as opposed to the ASCII representation in
+ hex characters, which is passed as "char *".
+
+You will see both of these things throughout the code.
+
+Now, for the meat:
+
+-----------------------------------------------------------------------------
+ case 0:
+ buf = read_object_with_reference(sha1, argv[1], &size, NULL);
+-----------------------------------------------------------------------------
+
+This is how you read a blob (actually, not only a blob, but any type of
+object). To know how the function `read_object_with_reference()` actually
+works, find the source code for it (something like `git grep
+read_object_with | grep ":[a-z]"` in the git repository), and read
+the source.
+
+To find out how the result can be used, just read on in `cmd_cat_file()`:
+
+-----------------------------------
+ write_or_die(1, buf, size);
+-----------------------------------
+
+Sometimes, you do not know where to look for a feature. In many such cases,
+it helps to search through the output of `git log`, and then `git show` the
+corresponding commit.
+
+Example: If you know that there was some test case for `git bundle`, but
+do not remember where it was (yes, you _could_ `git grep bundle t/`, but that
+does not illustrate the point!):
+
+------------------------
+$ git log --no-merges t/
+------------------------
+
+In the pager (`less`), just search for "bundle", go a few lines back,
+and see that it is in commit 18449ab0... Now just copy this object name,
+and paste it into the command line
+
+-------------------
+$ git show 18449ab0
+-------------------
+
+Voila.
+
+Another example: Find out what to do in order to make some script a
+builtin:
+
+-------------------------------------------------
+$ git log --no-merges --diff-filter=A builtin-*.c
+-------------------------------------------------
+
+You see, Git is actually the best tool to find out about the source of Git
+itself!
+
[[glossary]]
include::glossary.txt[]
^ permalink raw reply related
* [PATCHv2] connect: display connection progress
From: Michael S. Tsirkin @ 2007-05-10 9:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael S. Tsirkin, git
In-Reply-To: <7vhcqpbs78.fsf@assigned-by-dhcp.cox.net>
Make git notify the user about host resolution/connection attempts. This
is useful both as a progress indicator on slow links, and helps reassure the
user there are no DNS/firewall problems.
Signed-off-by: Michael S. Tsirkin <mst@dev.mellanox.co.il>
---
> > I find the following useful.
> > This currently only covers native git protocol. I expect it would
> > be easy to extend this to other protocols, if there's interest.
> > Opinions?
>
> Quoting Junio C Hamano <junkio@cox.net>:
> Subject: Re: [PATCH] connect: display connection progress
>
> "Michael S. Tsirkin" <mst@dev.mellanox.co.il> writes:
>
> I think giving this kind of feedback makes a lot of sense, from
> both the "assurance" point of view and also debuggability.
>
> But please do this only under verbose, or squelch it if "quiet"
> is asked.
Here's an updated patch. Please comment.
diff --git a/builtin-archive.c b/builtin-archive.c
index 7f4e409..5312e89 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -45,7 +45,7 @@ static int run_remote_archiver(const char *remote, int argc,
}
url = xstrdup(remote);
- pid = git_connect(fd, url, exec);
+ pid = git_connect(fd, url, exec, NET_QUIET);
if (pid < 0)
return pid;
diff --git a/cache.h b/cache.h
index 8e76152..232faa7 100644
--- a/cache.h
+++ b/cache.h
@@ -462,7 +462,8 @@ struct ref {
#define REF_HEADS (1u << 1)
#define REF_TAGS (1u << 2)
-extern pid_t git_connect(int fd[2], char *url, const char *prog);
+#define NET_QUIET (1u << 0)
+extern pid_t git_connect(int fd[2], char *url, const char *prog, int flags);
extern int finish_connect(pid_t pid);
extern int path_match(const char *path, int nr, char **match);
extern int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
diff --git a/connect.c b/connect.c
index da89c9c..fd4718a 100644
--- a/connect.c
+++ b/connect.c
@@ -394,7 +394,7 @@ static enum protocol get_protocol(const char *name)
/*
* Returns a connected socket() fd, or else die()s.
*/
-static int git_tcp_connect_sock(char *host)
+static int git_tcp_connect_sock(char *host, int flags)
{
int sockfd = -1, saved_errno = 0;
char *colon, *end;
@@ -425,10 +425,16 @@ static int git_tcp_connect_sock(char *host)
hints.ai_socktype = SOCK_STREAM;
hints.ai_protocol = IPPROTO_TCP;
+ if (!(flags & NET_QUIET))
+ fprintf(stderr, "Looking up %s ... ", host);
+
gai = getaddrinfo(host, port, &hints, &ai);
if (gai)
die("Unable to look up %s (port %s) (%s)", host, port, gai_strerror(gai));
+ if (!(flags & NET_QUIET))
+ fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
+
for (ai0 = ai; ai; ai = ai->ai_next) {
sockfd = socket(ai->ai_family,
ai->ai_socktype, ai->ai_protocol);
@@ -450,6 +456,9 @@ static int git_tcp_connect_sock(char *host)
if (sockfd < 0)
die("unable to connect a socket (%s)", strerror(saved_errno));
+ if (!(flags & NET_QUIET))
+ fprintf(stderr, "done.\n");
+
return sockfd;
}
@@ -458,7 +467,7 @@ static int git_tcp_connect_sock(char *host)
/*
* Returns a connected socket() fd, or else die()s.
*/
-static int git_tcp_connect_sock(char *host)
+static int git_tcp_connect_sock(char *host, int flags)
{
int sockfd = -1, saved_errno = 0;
char *colon, *end;
@@ -485,6 +494,9 @@ static int git_tcp_connect_sock(char *host)
port = colon + 1;
}
+ if (!(flags & NET_QUIET))
+ fprintf(stderr, "Looking up %s ... ", host);
+
he = gethostbyname(host);
if (!he)
die("Unable to look up %s (%s)", host, hstrerror(h_errno));
@@ -497,6 +509,9 @@ static int git_tcp_connect_sock(char *host)
nport = se->s_port;
}
+ if (!(flags & NET_QUIET))
+ fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
+
for (ap = he->h_addr_list; *ap; ap++) {
sockfd = socket(he->h_addrtype, SOCK_STREAM, 0);
if (sockfd < 0) {
@@ -521,15 +536,18 @@ static int git_tcp_connect_sock(char *host)
if (sockfd < 0)
die("unable to connect a socket (%s)", strerror(saved_errno));
+ if (!(flags & NET_QUIET))
+ fprintf(stderr, "done.\n");
+
return sockfd;
}
#endif /* NO_IPV6 */
-static void git_tcp_connect(int fd[2], char *host)
+static void git_tcp_connect(int fd[2], char *host, int flags)
{
- int sockfd = git_tcp_connect_sock(host);
+ int sockfd = git_tcp_connect_sock(host, flags);
fd[0] = sockfd;
fd[1] = dup(sockfd);
@@ -646,7 +664,7 @@ static void git_proxy_connect(int fd[2], char *host)
*
* Does not return a negative value on error; it just dies.
*/
-pid_t git_connect(int fd[2], char *url, const char *prog)
+pid_t git_connect(int fd[2], char *url, const char *prog, int flags)
{
char *host, *path = url;
char *end;
@@ -719,7 +737,7 @@ pid_t git_connect(int fd[2], char *url, const char *prog)
if (git_use_proxy(host))
git_proxy_connect(fd, host);
else
- git_tcp_connect(fd, host);
+ git_tcp_connect(fd, host, flags);
/*
* Separate original protocol components prog and path
* from extended components with a NUL byte.
diff --git a/fetch-pack.c b/fetch-pack.c
index 06f4aec..050b01d 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -733,7 +733,7 @@ int main(int argc, char **argv)
}
if (!dest)
usage(fetch_pack_usage);
- pid = git_connect(fd, dest, uploadpack);
+ pid = git_connect(fd, dest, uploadpack, quiet ? NET_QUIET : 0);
if (pid < 0)
return 1;
if (heads && nr_heads)
diff --git a/peek-remote.c b/peek-remote.c
index 96bfac4..a5e0fc1 100644
--- a/peek-remote.c
+++ b/peek-remote.c
@@ -64,7 +64,7 @@ int main(int argc, char **argv)
if (!dest || i != argc - 1)
usage(peek_remote_usage);
- pid = git_connect(fd, dest, uploadpack);
+ pid = git_connect(fd, dest, uploadpack, NET_QUIET);
if (pid < 0)
return 1;
ret = peek_remote(fd, flags);
diff --git a/send-pack.c b/send-pack.c
index d5b5162..5d99b25 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -393,7 +393,7 @@ int main(int argc, char **argv)
usage(send_pack_usage);
verify_remote_names(nr_heads, heads);
- pid = git_connect(fd, dest, receivepack);
+ pid = git_connect(fd, dest, receivepack, verbose ? 0 : NET_QUIET);
if (pid < 0)
return 1;
ret = send_pack(fd[0], fd[1], nr_heads, heads);
--
MST
^ permalink raw reply related
* Re: quick bare clones taking longer?
From: Johannes Sixt @ 2007-05-10 8:56 UTC (permalink / raw)
To: git
In-Reply-To: <vpqtzul3xzm.fsf@bauges.imag.fr>
Matthieu Moy wrote:
>
> Junio C Hamano <junkio@cox.net> writes:
>
> > get_repo_base() {
> > - (cd "$1" && (cd .git ; pwd)) 2> /dev/null
> > + (
> > + cd "`/bin/pwd`" &&
> > + cd "$1" &&
> > + (
> > + cd .git
> > + pwd
> > + )
> > + ) 2>/dev/null
> > }
>
> Will this work on windows?
Yes. As does the alternative that uses cd -P. MinGW uses bash (3.1
here).
-- Hannes
^ permalink raw reply
* Re: quick bare clones taking longer?
From: Matthieu Moy @ 2007-05-10 8:55 UTC (permalink / raw)
To: git
In-Reply-To: <7vejlpkruy.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> Is that a serious question?
It is. I have to admit that my knowledge about POSIX kind of things on
windows approaches zero, but a hardcoded /bin/something path sounds
suspicious to me.
Nothing more, nothing less in my question.
--
Matthieu
^ permalink raw reply
* Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.
From: Junio C Hamano @ 2007-05-10 8:43 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0705100421490.18541@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
> On Thu, 10 May 2007, Junio C Hamano wrote:
>
>> Daniel Barkalow <barkalow@iabervon.org> writes:
>>
>> >> And I think it does today.
>> >
>> > Hmm, and I guess URIs on the command line work the same way. How about
>> > requiring a '/' somewhere in a repository argument in order to treat it as
>> > a repository instead of a remote name? Then "../next-door-neighbour" would
>> > work, "./gitcvs.git" would work (in the odd case where you actually have a
>> > bare repository sitting in your working directory), but we'd avoid the
>> > current default of pushing to a bare repository in "./origin/" if nothing
>> > at all is configured.
>>
>> When I wrote the message you are responding to, I thought this
>> was a regression from the current behaviour, which (IIRC--it's
>> getting late and I am tired to double check) essentially says if
>> the token is a name of the directory, the target repository is a
>> local one, but "we'd avoid..." part seems to suggest that you
>> actually did this deliberately as a fix to some problem in the
>> current behaviour. I am not however sure what it exactly is.
>> Could you care to elaborate the part after "we'd avoid..." to
>> clarify what the problem is, please?
>
> The problem, in general, is that, if the remote name you specify (or
> "origin" if you don't specify any) is not configured as a remote, it is
> treated as a filename in the current directory for a local push. E.g.:
>
> $ git init
> $ git push
> fatal: 'origin': unable to chdir or not a git archive
> fatal: The remote end hung up unexpectedly
Ahh. You were trying to give it a better error message.
I think I lied in the previous message. I said we try to see if
it is a local directory name before using that name, but we do
not do it, and leave the error detection to the lower level on
the other side (push spawns send-pack which in turn spawns
receive-pack) instead.
Perhaps an alternative is to see if the name is configured as a
remote (if so, we obviously use it), and if not do stat() to see
if it is a directory (if so, use it as a local repository).
Then we do not have to impose new restriction of slash at all,
although it might complicate the code a bit more.
^ permalink raw reply
* Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.
From: Junio C Hamano @ 2007-05-10 8:35 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0705100355560.18541@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
> ..., "./gitcvs.git" would work (in the odd case where you actually have a
> bare repository sitting in your working directory), ...
This is a bit of tangent, because it does not change the issue
we are discussing, and I suspect you already know this.
But to avoid future confusion by people on the list who read
this in the archive...
"foo.git" does _NOT_ mean the directory is a bare
repository. It is perfectly normal to have "foo.git/"
that has a working tree whose repository data lives in
"foo.git/.git".
I _think_ (meaning, I haven't polled the userbase) requiring '/'
may not break people's existing setup too badly, while it _is_ a
regression in the sense that we suddenly start disallowing
something we allowed for a long time, perhaps without no
apparent good reason.
I hope you'll correct me about "without no apparent good reason"
part, which was what I was asking in my previous message. With
a good rationale, it is much easier to sell a change that is a
regression in the strictest sense but is unlikely to hurt people
in practice.
^ permalink raw reply
* Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.
From: Daniel Barkalow @ 2007-05-10 8:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7virb1ks1v.fsf@assigned-by-dhcp.cox.net>
On Thu, 10 May 2007, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> >> And I think it does today.
> >
> > Hmm, and I guess URIs on the command line work the same way. How about
> > requiring a '/' somewhere in a repository argument in order to treat it as
> > a repository instead of a remote name? Then "../next-door-neighbour" would
> > work, "./gitcvs.git" would work (in the odd case where you actually have a
> > bare repository sitting in your working directory), but we'd avoid the
> > current default of pushing to a bare repository in "./origin/" if nothing
> > at all is configured.
>
> When I wrote the message you are responding to, I thought this
> was a regression from the current behaviour, which (IIRC--it's
> getting late and I am tired to double check) essentially says if
> the token is a name of the directory, the target repository is a
> local one, but "we'd avoid..." part seems to suggest that you
> actually did this deliberately as a fix to some problem in the
> current behaviour. I am not however sure what it exactly is.
> Could you care to elaborate the part after "we'd avoid..." to
> clarify what the problem is, please?
The problem, in general, is that, if the remote name you specify (or
"origin" if you don't specify any) is not configured as a remote, it is
treated as a filename in the current directory for a local push. E.g.:
$ git init
$ git push
fatal: 'origin': unable to chdir or not a git archive
fatal: The remote end hung up unexpectedly
It's actually trying to push to ./origin/, which is totally nuts as a
default repository to push to. Similarly, if you typo an actual remote
name. Furthermore, builtin-push.c has an error message for the situation
where the repository specification is wrong, suggesting that there is
some invalid repository specification, but it isn't reachable. And it
carefully prevents remote names from starting with a '/', suggestion that
that is the distinguishing characteristic between directly-specified
repository URIs and configured remotes (which can't really be right, of
course).
I think the right answer is to say that configured remotes cannot contain
slashes, and directly-specified URIs must contain slashes, and it'll all
be clear.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: quick bare clones taking longer?
From: Junio C Hamano @ 2007-05-10 8:25 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqtzul3xzm.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> Junio C Hamano <junkio@cox.net> writes:
>
>> get_repo_base() {
>> - (cd "$1" && (cd .git ; pwd)) 2> /dev/null
>> + (
>> + cd "`/bin/pwd`" &&
>> + cd "$1" &&
>> + (
>> + cd .git
>> + pwd
>> + )
>> + ) 2>/dev/null
>> }
>
> Will this work on windows?
Is that a serious question?
If so, my answer is "I do not know, but the update is not any
more complex than the existing code -- both are perfectly fine
POSIX shell". Besides, if there are enough users who care about
Windows, there must be some competent ones among them, and we
will hear from them soon enough with an improvement patch.
If not, welcome to my killfile ;-).
NB. No, the last one is not serious. I do not have a
killfile.
^ permalink raw reply
* Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.
From: Junio C Hamano @ 2007-05-10 8:21 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0705100355560.18541@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
>> And I think it does today.
>
> Hmm, and I guess URIs on the command line work the same way. How about
> requiring a '/' somewhere in a repository argument in order to treat it as
> a repository instead of a remote name? Then "../next-door-neighbour" would
> work, "./gitcvs.git" would work (in the odd case where you actually have a
> bare repository sitting in your working directory), but we'd avoid the
> current default of pushing to a bare repository in "./origin/" if nothing
> at all is configured.
When I wrote the message you are responding to, I thought this
was a regression from the current behaviour, which (IIRC--it's
getting late and I am tired to double check) essentially says if
the token is a name of the directory, the target repository is a
local one, but "we'd avoid..." part seems to suggest that you
actually did this deliberately as a fix to some problem in the
current behaviour. I am not however sure what it exactly is.
Could you care to elaborate the part after "we'd avoid..." to
clarify what the problem is, please?
^ permalink raw reply
* Re: quick bare clones taking longer?
From: Matthieu Moy @ 2007-05-10 8:05 UTC (permalink / raw)
To: git
In-Reply-To: <7vd519r10c.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> get_repo_base() {
> - (cd "$1" && (cd .git ; pwd)) 2> /dev/null
> + (
> + cd "`/bin/pwd`" &&
> + cd "$1" &&
> + (
> + cd .git
> + pwd
> + )
> + ) 2>/dev/null
> }
Will this work on windows?
--
Matthieu
^ permalink raw reply
* Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.
From: Daniel Barkalow @ 2007-05-10 8:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmz0dktdf.fsf@assigned-by-dhcp.cox.net>
On Thu, 10 May 2007, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> > On Thu, 10 May 2007, Junio C Hamano wrote:
> >
> >> This seems to break t9400, with "fatal: bad repository 'gitcvs.git",
> >> upon "git push".
> >>
> >> : gitster t/db/remote; sh t9400-git-cvsserver-server.sh -i -v
> >> * expecting success: cvs -Q co -d cvswork master &&
> >> test "$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5))" = "empty/1.1/"
> >> cvs checkout: Updating cvswork
> >> U cvswork/empty
> >> * ok 1: basic checkout
> >>
> >> * expecting success: echo testfile1 >testfile1 &&
> >> git add testfile1 &&
> >> git commit -q -m "Add testfile1" &&
> >> git push gitcvs.git >/dev/null &&
> >
> > The man page doesn't think this is valid, since it only claims absolute
> > paths to work for local repositories.
>
> Does it? I suspect we need to fix the manpage then, as it is
> fairly common to do
>
> $ git fetch ../next-door-neighbour
>
> and expect the opposite to work as well.
>
> And I think it does today.
Hmm, and I guess URIs on the command line work the same way. How about
requiring a '/' somewhere in a repository argument in order to treat it as
a repository instead of a remote name? Then "../next-door-neighbour" would
work, "./gitcvs.git" would work (in the odd case where you actually have a
bare repository sitting in your working directory), but we'd avoid the
current default of pushing to a bare repository in "./origin/" if nothing
at all is configured.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [FAQ?] Rationale for git's way to manage the index
From: Matthieu Moy @ 2007-05-10 8:00 UTC (permalink / raw)
To: git
In-Reply-To: <alpine.LFD.0.98.0705091934440.4062@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Yeah, I don't think "git add -i" is a horrible flow - it just shouldn't be
> the only or the primary one (ie apparently it *is* the primary one for
> darcs, and that's a mistake!)
Note that darcs has a way to test before commit even for partial
commits. It re-creates your working tree, hardlinking unmodified
files, and runs a command there as a precommit hook.
I still prefer the old good "you commit what's in the tree, and run
whatever you want before commit", but their approach seems interesting
also in this case.
--
Matthieu
^ permalink raw reply
* Re: git rebase chokes on directory -> symlink -> directory
From: Johannes Sixt @ 2007-05-10 7:55 UTC (permalink / raw)
To: git
In-Reply-To: <7vmz0dmb2u.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> * git-rebase with -m is dog slow. There were people who
> advocated to make it the default, but they probably are
> either working in a very small project, or working on a
> filesystem that even git-apply is slow that the speed
> difference does not matter to them.
Heh... you name it. But just yesterday rebase -m was a life saver for me
because of its rename detection.
-- Hannes
^ permalink raw reply
* Re: [PATCH 1/3] Move remote parsing into a library file out of builtin-push.
From: Junio C Hamano @ 2007-05-10 7:52 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0705100328260.18541@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
> On Thu, 10 May 2007, Junio C Hamano wrote:
>
>> This seems to break t9400, with "fatal: bad repository 'gitcvs.git",
>> upon "git push".
>>
>> : gitster t/db/remote; sh t9400-git-cvsserver-server.sh -i -v
>> * expecting success: cvs -Q co -d cvswork master &&
>> test "$(echo $(grep -v ^D cvswork/CVS/Entries|cut -d/ -f2,3,5))" = "empty/1.1/"
>> cvs checkout: Updating cvswork
>> U cvswork/empty
>> * ok 1: basic checkout
>>
>> * expecting success: echo testfile1 >testfile1 &&
>> git add testfile1 &&
>> git commit -q -m "Add testfile1" &&
>> git push gitcvs.git >/dev/null &&
>
> The man page doesn't think this is valid, since it only claims absolute
> paths to work for local repositories.
Does it? I suspect we need to fix the manpage then, as it is
fairly common to do
$ git fetch ../next-door-neighbour
and expect the opposite to work as well.
And I think it does today.
^ 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