* Re: [PATCH 2/2] fetch/push: readd rsync support
From: Shawn O. Pearce @ 2007-09-28 5:20 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: gitster, Daniel Barkalow, git
In-Reply-To: <Pine.LNX.4.64.0709280606530.28395@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> +static int disconnect_rsync(struct transport *transport)
> +{
> + return 0;
> +}
>
> /* Generic functions for using commit walkers */
>
> @@ -402,7 +730,10 @@ struct transport *transport_get(struct remote *remote, const char *url)
> ret->url = url;
>
> if (!prefixcmp(url, "rsync://")) {
> - /* not supported; don't populate any ops */
> + ret->get_refs_list = get_refs_via_rsync;
> + ret->fetch = fetch_objs_via_rsync;
> + ret->push = rsync_transport_push;
> + ret->disconnect = disconnect_rsync;
>
> } else if (!prefixcmp(url, "http://")
> || !prefixcmp(url, "https://")
For what it's worth disconnect is an optional operation. You did
not need to implement it if you don't allocate a data member in
the struct transport. So removing disconnect_rsync() could save
you 6 lines or so.
I see push is now supported again. Didn't we remove rsync push
support a long time ago? Like say in:
commit c485104741ccdf32dd0c96fcb886c38a0b5badbd
Author: c.shoemaker@cox.net <c.shoemaker@cox.net>
Date: Sat Oct 29 00:16:33 2005 -0400
Add usage help to git-push.sh
Also clarify failure to push to read-only remote. Especially,
state why rsync:// is not used for pushing.
[jc: ideally rsync should not be used for anything]
Signed-off-by: Chris Shoemaker <c.shoemaker at cox.net>
Signed-off-by: Junio C Hamano <junkio@cox.net>
I guess it is nice to see that you can't kill rsync. Like Windows
it always finds it way back into your life.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Merge non-first refs that match first refspec
From: Shawn O. Pearce @ 2007-09-28 5:25 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0709280108060.5926@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> wrote:
> On Fri, 28 Sep 2007, Shawn O. Pearce wrote:
>
> > Daniel Barkalow <barkalow@iabervon.org> wrote:
> > > Beats me; Junio, what's your test case?
> >
> > If I understood him correctly it is this:
> >
> > mkdir foo; cd foo; git init
> > git config remote.origin.url git://repo.or.cz/alt-git.git
> > git config remote.origin.fetch refs/heads/master:refs/remotes/origin/master
> > git config --add remote.origin.fetch refs/heads/maint:refs/remotes/origin/maint
> > git pull
> >
> > We should see "master" listed in .git/FETCH_HEAD as a "for-merge"
> > and "maint" listed as a "not-for-merge"...
> >
> > But if that remote.origin.fetch was a wildcard spec this shouldn't
> > happen as the results are unpredictable. But above the user
> > explicitly put master first, so it should be defaulted to.
>
> But after that sequence, the right thing does happen. So I'm guessing that
> he has some different sequence that triggers a bug.
OK. Then I don't understand what issue Junio identified. And it
makes me feel better knowing that the current code does do the
right thing in the above case, because that's what I meant for it
to do when I was last in there...
--
Shawn.
^ permalink raw reply
* Re: Mergetool generating blank files (1.5.3)
From: David Kastrup @ 2007-09-28 6:19 UTC (permalink / raw)
To: Kelvie Wong; +Cc: Theodore Tso, Junio C Hamano, git
In-Reply-To: <94ccbe710709272117s6dee1a8jad6edf71dfb13c81@mail.gmail.com>
"Kelvie Wong" <kelvie@ieee.org> writes:
> On 9/27/07, Theodore Tso <tytso@mit.edu> wrote:
>
>> It's not that emacs sets $PWD via its first argument, but the output
>> file is passed from emerge-files*-command to stashed in the per-buffer
>> variable emerge-file-out, which in turn gets passed to the emacs lisp
>> file write-file, which is what gets run when you run C-x C-w --- and
>> write-file interprets a relative pathname based on the containing
>> directory of the existing buffer.
>> - Ted
>>
>
> Ah yes, I just started reading up on elisp a little while ago :)
>
> I'd always assumed that emacs kept an internal "pwd" variable (i.e.
> what's displayed with M-x pwd), but I guess my way of thinking is
> archaic and deprecated :(
It does. For every buffer.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: backup or mirror a repository
From: Junio C Hamano @ 2007-09-28 6:27 UTC (permalink / raw)
To: Dan Farina; +Cc: Johannes Schindelin, git
In-Reply-To: <1190947063.2263.46.camel@Tenacity>
Dan Farina <drfarina@gmail.com> writes:
> I did look at prune and update, but my problem is the opposite: I want
> something that will remove branches from the remote repo when they no
> longer exist locally. As-is over time I will proliferate little local
> branches unless I occasionally sit down and delete branches by operating
> directly on the bare backup repository. (and then use prune on the
> remote nodes)
The "git remote add --mirror" setup is about setting up the
local repository _AS_ the backup of the remote. In other words,
the contents come from the remote by fetching from it and safely
kept away from disaster on the local side. And for that,
"remote prune" is a perfect thing to do.
I think what you are asking for is an opposite, a backup remote
site you would push into. That is not what "remote add --mirror"
is about.
You can almost do it with
git push --all $remote
except there is no way to automagically remove the branch you
removed from the local repository. For that, we would need a
new --mirror option to "git-push".
I think it is trivial to do for native transports, as we first
get the list of all refs from the remote side before starting
the transfer. You need to change the last parameter called
'all' to remote.c::match_refs() into an enum ('push_all' being
one of choices), introduce another enum 'push_mirror', and teach
it to "match" the remote (i.e. dst) ref that does not have
corresponding entry on our side (i.e. src) with an empty object
name to mark it removed. Then the part marked as "Finally, tell
the other end!" in send-pack.c::send_pack() will take care of
the actual removal.
^ permalink raw reply
* Re: Mergetool generating blank files (1.5.3)
From: Pierre Habouzit @ 2007-09-28 6:35 UTC (permalink / raw)
To: Peter Baumann; +Cc: git, Russ Brown, Kelvie Wong
In-Reply-To: <20070928051503.GA19815@xp.machine.xx>
[-- Attachment #1: Type: text/plain, Size: 1062 bytes --]
On Fri, Sep 28, 2007 at 05:15:03AM +0000, Peter Baumann wrote:
> FWIW, xxdiff has support to handle halfway merged files, so that if git
> could merge some hunks already for you (e.g. rerere kicked in), you
> don't have to redo the _whole_ merge by hand, just call
>
> xxdiff -U file/with/mergemarkers/inside
>
> and it will do the right thing. Not sure if the other tools could handle
> it, but any pointers appreciated, because it often happens to me that
> only one hunk out of several wasn't merged automatically by git. And
> mergetool wants to always redo the whole merge, which isn't the best it
> can do.
For what I've seen, it's how meld works, and I find it nice too. Meld
is quite slow (python + gnomeish doesn't help) but well, I don't merge
things that often, so like said I think I'll give it a try for a while
and see if it has what it takes :)
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: git push (mis ?)behavior
From: Steffen Prohaska @ 2007-09-28 6:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pierre Habouzit, git
In-Reply-To: <7v3awzvrpr.fsf@gitster.siamese.dyndns.org>
On Sep 27, 2007, at 9:22 PM, Junio C Hamano wrote:
>
> So what's the desired semantics?
>
> The current semantics is:
>
> "git push" says "you do not say to which repository?" and
> consults "branch.<current>.remote" but defaults to 'origin'
> if unconfigured.
>
> "git push <name>" (or using the <name> determined as above)
> says "you do not say which branches?" and consults
> "remote.<name>.push" to find branches to push out, but
> defaults to 'matching branches' if unconfigured.
>
> What you would want to change is the fallback behaviour for
> unconfigured "remote.<name>.push". I think it is sensible to
> have an option to make it push only the current branch.
I'm not sure that changing the fallback behaviour for unconfigured
"remote.<name>.push" is sufficient.
When "remote.<name>.push" is set I'd expect "git push" to
choose only the 'right' remote.<name>.push lines, that is
the lines that have the current branch as the local ref.
"git push" would only push the current branch, which could be pushed
to 0 or more branches on the remote side. If no "remote.<name>.push"
contains the current branch as a local ref nothing would happen
(maybe a warning?). If several "remote.<name>.push" have the current
branch as the local ref the branch would be pushed to several
remote branches. But other branches than the current branch
would _never_ be pushed if no argument is given to 'git push'.
Steffen
^ permalink raw reply
* Re: git push (mis ?)behavior
From: Pierre Habouzit @ 2007-09-28 6:58 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: Junio C Hamano, git
In-Reply-To: <9D61974D-E08D-49F6-9C88-6BE446D53C74@zib.de>
[-- Attachment #1: Type: text/plain, Size: 2260 bytes --]
On Fri, Sep 28, 2007 at 06:52:47AM +0000, Steffen Prohaska wrote:
>
> On Sep 27, 2007, at 9:22 PM, Junio C Hamano wrote:
>
> >
> >So what's the desired semantics?
> >
> >The current semantics is:
> >
> > "git push" says "you do not say to which repository?" and
> > consults "branch.<current>.remote" but defaults to 'origin'
> > if unconfigured.
> >
> > "git push <name>" (or using the <name> determined as above)
> > says "you do not say which branches?" and consults
> > "remote.<name>.push" to find branches to push out, but
> > defaults to 'matching branches' if unconfigured.
> >
> >What you would want to change is the fallback behaviour for
> >unconfigured "remote.<name>.push". I think it is sensible to
> >have an option to make it push only the current branch.
>
> I'm not sure that changing the fallback behaviour for unconfigured
> "remote.<name>.push" is sufficient.
>
> When "remote.<name>.push" is set I'd expect "git push" to
> choose only the 'right' remote.<name>.push lines, that is
> the lines that have the current branch as the local ref.
> "git push" would only push the current branch, which could be pushed
> to 0 or more branches on the remote side. If no "remote.<name>.push"
> contains the current branch as a local ref nothing would happen
> (maybe a warning?). If several "remote.<name>.push" have the current
> branch as the local ref the branch would be pushed to several
> remote branches. But other branches than the current branch
> would _never_ be pushed if no argument is given to 'git push'.
I'm not really sure that it makes sense, as by default, git push won't
create a new remote ref. So unless you have a branch that matches some
remote ref, but that you never want to push, even when on it and typing
git push... there is nothing that could happen by mistake.
So if you don't want to push such a branch, ever, then you should IMHO
not name it in a way that it matches a refspec in the first place, and
be safe.
So I do not believe we need that extra complexity.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: git push (mis ?)behavior
From: Junio C Hamano @ 2007-09-28 7:07 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: Pierre Habouzit, git
In-Reply-To: <9D61974D-E08D-49F6-9C88-6BE446D53C74@zib.de>
Steffen Prohaska <prohaska@zib.de> writes:
> When "remote.<name>.push" is set I'd expect "git push" to
> choose only the 'right' remote.<name>.push lines, that is
> the lines that have the current branch as the local ref.
That would break the existing setup so it would not fly as a
default, although it could be added as an option.
^ permalink raw reply
* Re: backup or mirror a repository
From: Dan Farina @ 2007-09-28 7:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vsl4zqp8l.fsf@gitster.siamese.dyndns.org>
On Thu, 2007-09-27 at 23:27 -0700, Junio C Hamano wrote:
> You can almost do it with
>
> git push --all $remote
>
> except there is no way to automagically remove the branch you
> removed from the local repository. For that, we would need a
> new --mirror option to "git-push".
Yup! I actually was looking for such a thing in the git-push man page,
but was unsuccessful. I was living with git-push --all for a little
while before I thought I'd ask. Unfortunately, the preferred setup
(having the backup machine actively perform fetch and prune) is not very
nice for me due to firewalls.
>
> I think it is trivial to do for native transports, as we first
> get the list of all refs from the remote side before starting
> the transfer. You need to change the last parameter called
> 'all' to remote.c::match_refs() into an enum ('push_all' being
> one of choices), introduce another enum 'push_mirror', and teach
> it to "match" the remote (i.e. dst) ref that does not have
> corresponding entry on our side (i.e. src) with an empty object
> name to mark it removed. Then the part marked as "Finally, tell
> the other end!" in send-pack.c::send_pack() will take care of
> the actual removal.
I should have a look, but if someone else wants to work on this they
shouldn't block on me: work already made me get hit by a bus on one
project (luckily I wasn't important), and the storm is not over.
Thanks,
fdr
^ permalink raw reply
* Re: Mergetool generating blank files (1.5.3)
From: David Kågedal @ 2007-09-28 8:43 UTC (permalink / raw)
To: git
In-Reply-To: <94ccbe710709271224rc65b6f4k8b68419629ed5b45@mail.gmail.com>
"Kelvie Wong" <kelvie@ieee.org> writes:
> I've tried all of the ones that were supported, the result is the same
> -- blank files in all three windows.
>
> It is because git mergetool fails to generate these files for whatever
> reason (the filebasename.{REMOTE,LOCAL,BASE}.* files). I don't know
> why this happens.
>
> As for merge utilities, all I need is something that looks for the
> first <<<<<, and lets me choose which version I want (either top or
> bottom), plain and simple :/ I don't even need/want a gui.
Since you are already using Emacs, let me suggest you use smerge
(together with ediff).
Use M-x smerge-mode and go over the conflicts and select which one you
want with C-c ^ m (for "mine") or C-c ^ o (for "other") or one of the
commands. Or press C-c ^ E to run a three-window merge.
Or use the last directly by running M-x smerge-ediff.
--
David Kågedal
^ permalink raw reply
* Re: [PATCH] alloc_ref(): allow for trailing NUL
From: Junio C Hamano @ 2007-09-28 8:46 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Johannes Schindelin, gitster, git
In-Reply-To: <Pine.LNX.4.64.0709280046241.5926@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
> On Fri, 28 Sep 2007, Johannes Schindelin wrote:
>
>> The parameter name "namelen" suggests that you pass the equivalent of
>> strlen() to the function alloc_ref(). However, this function did not
>> allocate enough space to put a NUL after the name.
>>
>> Since struct ref does not have any member to describe the length of the
>> string, this just does not make sense.
>>
>> So make space for the NUL.
>
> Good point, but shouldn't you then fix call sites that use strlen(name) +
> 1?
Good point.
I audited "git grep -A2 -B4 -e alloc_ref next master" output,
and it appears almost everybody knows alloc_ref() wants the
caller to count the terminating NUL.
There however are a few gotchas.
* There is one overallocation in connect.c, which would not
hurt but is wasteful;
* next:transport.c has alloc_ref(strlen(e->name)) which is a
no-no;
Discarding Johannes's patch, the following would fix it.
---
connect.c | 4 ++--
transport.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/connect.c b/connect.c
index 06d279e..3d5c4ab 100644
--- a/connect.c
+++ b/connect.c
@@ -72,9 +72,9 @@ struct ref **get_remote_heads(int in, struct ref **list,
continue;
if (nr_match && !path_match(name, nr_match, match))
continue;
- ref = alloc_ref(len - 40);
+ ref = alloc_ref(name_len + 1);
hashcpy(ref->old_sha1, old_sha1);
- memcpy(ref->name, buffer + 41, len - 40);
+ memcpy(ref->name, buffer + 41, name_len + 1);
*list = ref;
list = &ref->next;
}
diff --git a/transport.c b/transport.c
index 4f9cddc..3475cca 100644
--- a/transport.c
+++ b/transport.c
@@ -215,7 +215,7 @@ static struct ref *get_refs_from_bundle(const struct transport *transport)
die ("Could not read bundle '%s'.", transport->url);
for (i = 0; i < data->header.references.nr; i++) {
struct ref_list_entry *e = data->header.references.list + i;
- struct ref *ref = alloc_ref(strlen(e->name));
+ struct ref *ref = alloc_ref(strlen(e->name) + 1);
hashcpy(ref->old_sha1, e->sha1);
strcpy(ref->name, e->name);
ref->next = result;
^ permalink raw reply related
* Re: [PATCH 1/2] Introduce remove_dir_recursively()
From: Junio C Hamano @ 2007-09-28 9:05 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: gitster, Daniel Barkalow, git
In-Reply-To: <Pine.LNX.4.64.0709280606350.28395@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> +int remove_dir_recursively(char *path, int len, int only_empty)
> +{
> ...
> + namlen = strlen(e->d_name);
> + if (len + namlen > PATH_MAX ||
> + !memcpy(path + len, e->d_name, namlen) ||
> + (path[len + namlen] = '\0') ||
> + lstat(path, &st))
> + ; /* fall thru */
> + else if (S_ISDIR(st.st_mode)) {
> + if (!remove_dir_recursively(path, len + namlen,
> + only_empty))
> + continue; /* happy */
> + } else if (!only_empty &&
> + len + namlen + 1 < PATH_MAX &&
> + !unlink(path))
> + continue; /* happy, too */
> +
> + /* path too long, stat fails, or non-directory still exists */
> + ret = -1;
> + break;
Is it only me who finds the first if () condition way too
convoluted and needs to read three times to convince oneself
that it is doing a sane thing?
Please, especially...
* For $DEITY's sake, memcpy() returns pointer to dst which you
know is not NULL. so !memcpy() is always false here, which
might be _convenient_ for you and the compiler but not for
a human reader of the code who needs to blink twice wondering
if you meant !memcmp().
* Same for (path[] = '\0'), wondering if it is misspelled
(path[] == '\0').
^ permalink raw reply
* Re: git push (mis ?)behavior
From: Steffen Prohaska @ 2007-09-28 9:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pierre Habouzit, git
In-Reply-To: <7vodfnqndc.fsf@gitster.siamese.dyndns.org>
On Sep 28, 2007, at 9:07 AM, Junio C Hamano wrote:
> Steffen Prohaska <prohaska@zib.de> writes:
>
>> When "remote.<name>.push" is set I'd expect "git push" to
>> choose only the 'right' remote.<name>.push lines, that is
>> the lines that have the current branch as the local ref.
>
> That would break the existing setup so it would not fly as a
> default, although it could be added as an option.
Do you mean 'not now' or never, i.e. not in git 1.6?
What does 'break the existing setup' means? Pushing all branches
that are configured in "remote.<name>.push" could be done by
"git push <name>". This would be in the line with the idea that
"git push" should only operate on the current branch and operations
involving other local refs should be explicitly stated.
Steffen
^ permalink raw reply
* git-mergetool
From: Mattias Ulbrich @ 2007-09-28 8:56 UTC (permalink / raw)
To: git
Hello everybody,
There might by a tiny typo-bug in the git-mergetool:
-----------
case "$merge_tool" in
kdiff3)
if base_present ; then
(kdiff3 --auto --L1 "$path (Base)" -L2 "$path (Local)" --L3 "$path (Remote)"
\
-o "$path" -- "$BASE" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
else
(kdiff3 --auto -L1 "$path (Local)" --L2 "$path (Remote)" \
-o "$path" -- "$LOCAL" "$REMOTE" > /dev/null 2>&1)
fi
-----------
In the first call to kdiff3 the argument is only -L2 but should be --L2 (two
minus signs) like in the second call - where -L1 misses its second "-".
Thank you for a very mighty set of tool
Sincerely
Mattias
--
Mattias Ulbrich
Forschungsgruppe Prof. Dr. P.H. Schmitt
Institut für Theoretische Informatik
Universität Karlsruhe
^ permalink raw reply
* Re: [PATCH 2/2] fetch/push: readd rsync support
From: Junio C Hamano @ 2007-09-28 9:30 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Daniel Barkalow, git
In-Reply-To: <Pine.LNX.4.64.0709280606530.28395@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> +/*
> + * path is assumed to point to a buffer of PATH_MAX bytes, and
> + * path + name_offset is expected to point to "refs/".
> + */
> +
> +static int read_loose_refs(char *path, int name_offset, struct ref **tail)
> +{
> + DIR *dir = opendir(path);
> + struct dirent *de;
> + struct {
> + struct dirent *entries;
> + int nr, alloc;
> + } list;
> + int i, pathlen;
> +
> + if (!dir)
> + return -1;
> +
> + memset (&list, 0, sizeof(list));
> +
> + while ((de = readdir(dir))) {
> + if (de->d_name[0] == '.' && (de->d_name[1] == '\0' ||
> + (de->d_name[1] == '.' &&
> + de->d_name[2] == '\0')))
> + continue;
> + if (list.nr >= list.alloc) {
> + list.alloc = alloc_nr(list.nr);
> + list.entries = xrealloc(list.entries,
> + list.alloc * sizeof(*de));
> + }
ALLOC_GROW() not applicable here?
> + list.entries[list.nr++] = *de;
Are you sure about this?
The last paragraph in Rationale section, in
http://www.opengroup.org/onlinepubs/000095399/basedefs/dirent.h.html
suggests that the d_name[] member in struct dirent could be
declared at the very end of the structure as length of 1 (the
traditional trick to implement a flex-array); your assignment
from *de into entries[] would not work as expected on such an
implementation.
On Linux with glibc it appears bits/dirent.h defines dirent with
"char d_name[256]", so you may not see a breakage there, though.
You only use a list of strings (char **), don't you?
> ...
> + if (fd < 0)
> + continue;
> + next = alloc_ref(strlen(path + name_offset));
And as we discussed earlier you would need one more byte here ;-).
^ permalink raw reply
* Re: git push (mis ?)behavior
From: Steffen Prohaska @ 2007-09-28 9:26 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Junio C Hamano, git
In-Reply-To: <20070928065823.GB19299@artemis.corp>
On Sep 28, 2007, at 8:58 AM, Pierre Habouzit wrote:
> On Fri, Sep 28, 2007 at 06:52:47AM +0000, Steffen Prohaska wrote:
>>
>> On Sep 27, 2007, at 9:22 PM, Junio C Hamano wrote:
>>
>>>
>>> So what's the desired semantics?
>>>
>>> The current semantics is:
>>>
>>> "git push" says "you do not say to which repository?" and
>>> consults "branch.<current>.remote" but defaults to 'origin'
>>> if unconfigured.
>>>
>>> "git push <name>" (or using the <name> determined as above)
>>> says "you do not say which branches?" and consults
>>> "remote.<name>.push" to find branches to push out, but
>>> defaults to 'matching branches' if unconfigured.
>>>
>>> What you would want to change is the fallback behaviour for
>>> unconfigured "remote.<name>.push". I think it is sensible to
>>> have an option to make it push only the current branch.
>>
>> I'm not sure that changing the fallback behaviour for unconfigured
>> "remote.<name>.push" is sufficient.
>>
>> When "remote.<name>.push" is set I'd expect "git push" to
>> choose only the 'right' remote.<name>.push lines, that is
>> the lines that have the current branch as the local ref.
>> "git push" would only push the current branch, which could be pushed
>> to 0 or more branches on the remote side. If no "remote.<name>.push"
>> contains the current branch as a local ref nothing would happen
>> (maybe a warning?). If several "remote.<name>.push" have the current
>> branch as the local ref the branch would be pushed to several
>> remote branches. But other branches than the current branch
>> would _never_ be pushed if no argument is given to 'git push'.
>
> I'm not really sure that it makes sense, as by default, git push
> won't
> create a new remote ref. So unless you have a branch that matches some
> remote ref, but that you never want to push, even when on it and
> typing
> git push... there is nothing that could happen by mistake.
>
> So if you don't want to push such a branch, ever, then you should
> IMHO
> not name it in a way that it matches a refspec in the first place, and
> be safe.
I could have a couple of local branches typically pushed to a couple of
remote branches. "git push origin" would update all remote refs.
But I may also be interested to push only the current branch I'm working
on. I may also have some pending fixes on another branch that should not
be pushed now. Currently I need to do "git push origin
<current>:<someremote>".
Let me put it as a question: How can I push changes from the current
branch to all remote refs it is configured to push to via
"remote.<name>.push"
without pushing anything else at the same time?
Steffen
^ permalink raw reply
* Re: [PATCH] Merge non-first refs that match first refspec
From: Junio C Hamano @ 2007-09-28 9:34 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Shawn O. Pearce, git
In-Reply-To: <Pine.LNX.4.64.0709280026240.5926@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
> Beats me; Junio, what's your test case?
I can paste tomorrow (it is a clone of git.git at work). I do
not use .git/config but .git/remotes/origin and explicit four
separate Pull: lines and going over http.
^ permalink raw reply
* Re: git push (mis ?)behavior
From: Junio C Hamano @ 2007-09-28 9:44 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: Pierre Habouzit, git
In-Reply-To: <09A90525-8B0B-4249-904C-722BCC544B4E@zib.de>
Steffen Prohaska <prohaska@zib.de> writes:
> On Sep 28, 2007, at 8:58 AM, Pierre Habouzit wrote:
>
>> On Fri, Sep 28, 2007 at 06:52:47AM +0000, Steffen Prohaska wrote:
>
> But I may also be interested to push only the current branch I'm working
> on. I may also have some pending fixes on another branch that should not
> be pushed now. Currently I need to do "git push origin
> <current>:<someremote>".
Sounds sensible.
> Let me put it as a question: How can I push changes from the current
> branch to all remote refs it is configured to push to via
> "remote.<name>.push"
> without pushing anything else at the same time?
Sorry, but I do not get you. Are you talking about pushing your
'frotz' into more than one branches 'nitfol' and 'xyzzy' at the
same remote 'origin' without having to say
$ git push origin frotz:nitfol frotz:xyzzy
???
^ permalink raw reply
* Re: git push (mis ?)behavior
From: Steffen Prohaska @ 2007-09-28 10:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pierre Habouzit, git
In-Reply-To: <7vr6kjp1jw.fsf@gitster.siamese.dyndns.org>
On Sep 28, 2007, at 11:44 AM, Junio C Hamano wrote:
>> Let me put it as a question: How can I push changes from the current
>> branch to all remote refs it is configured to push to via
>> "remote.<name>.push"
>> without pushing anything else at the same time?
>
> Sorry, but I do not get you. Are you talking about pushing your
> 'frotz' into more than one branches 'nitfol' and 'xyzzy' at the
> same remote 'origin' without having to say
>
> $ git push origin frotz:nitfol frotz:xyzzy
>
> ???
Yes. Doesn't sound like a very reasonable workflow. But you can do
it with "remote.<name>.push". So I think "git push" should somehow
deal with it in a sensible way.
Steffen
^ permalink raw reply
* Re: [PATCH] alloc_ref(): allow for trailing NUL
From: Johannes Schindelin @ 2007-09-28 12:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Daniel Barkalow, git
In-Reply-To: <7vhclfqisq.fsf@gitster.siamese.dyndns.org>
Hi,
On Fri, 28 Sep 2007, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> > On Fri, 28 Sep 2007, Johannes Schindelin wrote:
> >
> >> The parameter name "namelen" suggests that you pass the equivalent of
> >> strlen() to the function alloc_ref(). However, this function did not
> >> allocate enough space to put a NUL after the name.
> >>
> >> Since struct ref does not have any member to describe the length of the
> >> string, this just does not make sense.
> >>
> >> So make space for the NUL.
> >
> > Good point, but shouldn't you then fix call sites that use strlen(name) +
> > 1?
>
> Good point.
>
> I audited "git grep -A2 -B4 -e alloc_ref next master" output,
> and it appears almost everybody knows alloc_ref() wants the
> caller to count the terminating NUL.
>
> There however are a few gotchas.
>
> * There is one overallocation in connect.c, which would not
> hurt but is wasteful;
>
> * next:transport.c has alloc_ref(strlen(e->name)) which is a
> no-no;
>
> Discarding Johannes's patch, the following would fix it.
But should the signature of alloc_ref() not be changed, then, to read
struct ref *alloc_ref(unsigned name_alloc);
Hm?
Further, I am quite sure that the same mistake will happen again, until we
change the function to get the name length, not the number of bytes to
allocate.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/2] fetch/push: readd rsync support
From: Johannes Schindelin @ 2007-09-28 12:22 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: gitster, Daniel Barkalow, git
In-Reply-To: <20070928052018.GW3099@spearce.org>
Hi,
On Fri, 28 Sep 2007, Shawn O. Pearce wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > +static int disconnect_rsync(struct transport *transport)
> > +{
> > + return 0;
> > +}
> >
> > /* Generic functions for using commit walkers */
> >
> > @@ -402,7 +730,10 @@ struct transport *transport_get(struct remote *remote, const char *url)
> > ret->url = url;
> >
> > if (!prefixcmp(url, "rsync://")) {
> > - /* not supported; don't populate any ops */
> > + ret->get_refs_list = get_refs_via_rsync;
> > + ret->fetch = fetch_objs_via_rsync;
> > + ret->push = rsync_transport_push;
> > + ret->disconnect = disconnect_rsync;
> >
> > } else if (!prefixcmp(url, "http://")
> > || !prefixcmp(url, "https://")
>
> For what it's worth disconnect is an optional operation.
Strange. When not assigning disconnect, I had an error last night. But
it could well be something independent.
Reading the code again, I should not even have to assign NULL to
ret->disconnect.
Will fix.
> I see push is now supported again. Didn't we remove rsync push support
> a long time ago? Like say in:
>
> commit c485104741ccdf32dd0c96fcb886c38a0b5badbd
> Author: c.shoemaker@cox.net <c.shoemaker@cox.net>
> Date: Sat Oct 29 00:16:33 2005 -0400
>
> Add usage help to git-push.sh
>
> Also clarify failure to push to read-only remote. Especially,
> state why rsync:// is not used for pushing.
>
> [jc: ideally rsync should not be used for anything]
>
> Signed-off-by: Chris Shoemaker <c.shoemaker at cox.net>
> Signed-off-by: Junio C Hamano <junkio@cox.net>
I know. I hesitated, but then: it was all too easy to have it in, so why
should we relinquish it?
> I guess it is nice to see that you can't kill rsync. Like Windows it
> always finds it way back into your life.
Hehe.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/2] Introduce remove_dir_recursively()
From: Johannes Schindelin @ 2007-09-28 12:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Daniel Barkalow, git
In-Reply-To: <7v8x6rqhwy.fsf@gitster.siamese.dyndns.org>
Hi,
On Fri, 28 Sep 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > +int remove_dir_recursively(char *path, int len, int only_empty)
> > +{
> > ...
> > + namlen = strlen(e->d_name);
> > + if (len + namlen > PATH_MAX ||
> > + !memcpy(path + len, e->d_name, namlen) ||
> > + (path[len + namlen] = '\0') ||
> > + lstat(path, &st))
> > + ; /* fall thru */
> > + else if (S_ISDIR(st.st_mode)) {
> > + if (!remove_dir_recursively(path, len + namlen,
> > + only_empty))
> > + continue; /* happy */
> > + } else if (!only_empty &&
> > + len + namlen + 1 < PATH_MAX &&
> > + !unlink(path))
> > + continue; /* happy, too */
> > +
> > + /* path too long, stat fails, or non-directory still exists */
> > + ret = -1;
> > + break;
>
> Is it only me who finds the first if () condition way too
> convoluted and needs to read three times to convince oneself
> that it is doing a sane thing?
>
> Please, especially...
>
> * For $DEITY's sake, memcpy() returns pointer to dst which you
> know is not NULL. so !memcpy() is always false here, which
> might be _convenient_ for you and the compiler but not for
> a human reader of the code who needs to blink twice wondering
> if you meant !memcmp().
>
> * Same for (path[] = '\0'), wondering if it is misspelled
> (path[] == '\0').
Okay, will fix (with an evil goto).
BTW it just hit me that this magic reliance on a buffer of size PATH_MAX
is not good at all. It even hit _me_ while developing that series.
So I'll change that to a strbuf, too. (Which will fix the convoluted
logic quite some, incidentally.)
Ciao,
Dscho
^ permalink raw reply
* Re: git push (mis ?)behavior
From: Wincent Colaiuta @ 2007-09-28 12:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pierre Habouzit, git
In-Reply-To: <7v3awzvrpr.fsf@gitster.siamese.dyndns.org>
El 27/9/2007, a las 21:22, Junio C Hamano escribió:
> What you would want to change is the fallback behaviour for
> unconfigured "remote.<name>.push". I think it is sensible to
> have an option to make it push only the current branch. I am
> not sure if it is sensible to make that the default (and
> introduce --matching option to get the current behaviour) at
> this point in 1.5.X series, but from the general usability point
> of view, I would not object to demote 'matching' to optional and
> make 'current only' the default in 1.6.X or later.
>
> Thoughts?
I'd certainly welcome this change for two reasons:
1. It makes the behaviour more conservative (that is, harder to do
something destructive and irreversible) by using the more limited
scope by default. If you make a mistake and see that you really meant
to push all matching branches then you can just do the push again
with that switch; compare that to the situation now where if you
accidentally push all matching branches when you only wanted to push
the current branch then there's no way for you to "unpush".
2. Mental baggage from working with SVK (where "push" means merge
changes back to the branch the current branch previously branched
from, and "pull" means merge changes into the current branch from the
branch you previously branched from). At least for me and I suspect
for many others the "current only" default in 1.6 or latter would be
less "surprising" than the current behaviour can be.
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH] alloc_ref(): allow for trailing NUL
From: Pierre Habouzit @ 2007-09-28 12:41 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Daniel Barkalow, git
In-Reply-To: <Pine.LNX.4.64.0709281259050.28395@racer.site>
[-- Attachment #1: Type: text/plain, Size: 2898 bytes --]
On Fri, Sep 28, 2007 at 12:01:28PM +0000, Johannes Schindelin wrote:
> But should the signature of alloc_ref() not be changed, then, to read
>
> struct ref *alloc_ref(unsigned name_alloc);
>
> Hm?
>
> Further, I am quite sure that the same mistake will happen again, until we
> change the function to get the name length, not the number of bytes to
> allocate.
<janitor cap on>
While being at it, I noticed that the following code pattern is used
in many places in git:
struct something *ptr = xmalloc(sizeof(struct something) + some_string_len + 1);
memset(ptr, 0, sizeof(struct something));
memcpy(ptr->name, somestring, some_string_len + 1);
With name being a flex array.
Maybe we could deal with all of these, and make a generic construction
that would take care of allocating the proper space
I believe we could have function doing that, that would factorize this
code, and use a nice api à la xmemdupz for this.
It would be something like:
void *xflexdupz(size_t offset, void *src, size_t len)
{
char *p = xmalloc(offset + len + 1);
memset(p, 0, offset);
memcpy(p + offset, src, len);
p[offset + len] = '\0';
return p;
}
Then alloc_ref could be a wrapper around:
xflexdupz(offsetof(struct ref, name), ..., ...).
Of course right now alloc_ref doesn't perform any kind of copy, but a
grep -A1 will convince you that it's not a problem:
$ grep -A1 alloc_ref **/*.[hc]
builtin-fetch.c: rm = alloc_ref(strlen(ref_name) + 1);
builtin-fetch.c- strcpy(rm->name, ref_name);
builtin-fetch.c: rm->peer_ref = alloc_ref(strlen(ref_name) + 1);
builtin-fetch.c- strcpy(rm->peer_ref->name, ref_name);
--
connect.c: ref = alloc_ref(len - 40);
connect.c- hashcpy(ref->old_sha1, old_sha1);
--
remote.c:struct ref *alloc_ref(unsigned namelen)
remote.c-{
--
remote.c: ref = alloc_ref(20);
remote.c- strcpy(ref->name, "(delete)");
--
remote.c: ref = alloc_ref(len);
remote.c- memcpy(ref->name, name, len);
--
remote.c: ret = alloc_ref(len);
remote.c- memcpy(ret->name, name, len);
--
remote.c: cpy->peer_ref = alloc_ref(local_prefix_len +
remote.c- strlen(match) + 1);
--
remote.c: ret = alloc_ref(strlen(name) + 1);
remote.c- strcpy(ret->name, name);
--
remote.c: ret = alloc_ref(strlen(name) + 6);
remote.c- sprintf(ret->name, "refs/%s", name);
--
remote.c: ret = alloc_ref(strlen(name) + 12);
remote.c- sprintf(ret->name, "refs/heads/%s", name);
--
remote.h:struct ref *alloc_ref(unsigned namelen);
remote.h-
--
transport.c: struct ref *ref = alloc_ref(strlen(e->name));
transport.c- hashcpy(ref->old_sha1, e->sha1);
</janitor>
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] fetch/push: readd rsync support
From: Johannes Schindelin @ 2007-09-28 12:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Daniel Barkalow, git
In-Reply-To: <7v3awzqgqc.fsf@gitster.siamese.dyndns.org>
Hi,
On Fri, 28 Sep 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > +/*
> > + * path is assumed to point to a buffer of PATH_MAX bytes, and
> > + * path + name_offset is expected to point to "refs/".
> > + */
> > +
> > +static int read_loose_refs(char *path, int name_offset, struct ref **tail)
> > +{
> > + DIR *dir = opendir(path);
> > + struct dirent *de;
> > + struct {
> > + struct dirent *entries;
> > + int nr, alloc;
> > + } list;
> > + int i, pathlen;
> > +
> > + if (!dir)
> > + return -1;
> > +
> > + memset (&list, 0, sizeof(list));
> > +
> > + while ((de = readdir(dir))) {
> > + if (de->d_name[0] == '.' && (de->d_name[1] == '\0' ||
> > + (de->d_name[1] == '.' &&
> > + de->d_name[2] == '\0')))
> > + continue;
> > + if (list.nr >= list.alloc) {
> > + list.alloc = alloc_nr(list.nr);
> > + list.entries = xrealloc(list.entries,
> > + list.alloc * sizeof(*de));
> > + }
>
> ALLOC_GROW() not applicable here?
>
> > + list.entries[list.nr++] = *de;
>
> Are you sure about this?
>
> The last paragraph in Rationale section, in
>
> http://www.opengroup.org/onlinepubs/000095399/basedefs/dirent.h.html
>
> suggests that the d_name[] member in struct dirent could be
> declared at the very end of the structure as length of 1 (the
> traditional trick to implement a flex-array); your assignment
> >from *de into entries[] would not work as expected on such an
> implementation.
>
> On Linux with glibc it appears bits/dirent.h defines dirent with
> "char d_name[256]", so you may not see a breakage there, though.
D'oh! You're completely right.
> You only use a list of strings (char **), don't you?
Originally, I wanted to use the d_type, too, but as I mentioned on IRC, it
is not reliable enough (shows DT_UNKNOWN _all_ the time here).
But yes, I'll redo it, using ALLOC_GROW.
> > ...
> > + if (fd < 0)
> > + continue;
> > + next = alloc_ref(strlen(path + name_offset));
>
> And as we discussed earlier you would need one more byte here ;-).
Well, not after my patch, which I thought of as (1/3), but then decided it
is good enough to be (1/1) until you shot it down...
Will fix.
Ciao,
Dscho
^ 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