Git development
 help / color / mirror / Atom feed
* Re: Mergetool generating blank files (1.5.3)
From: Kelvie Wong @ 2007-09-27 21:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwsubsuh1.fsf@gitster.siamese.dyndns.org>

On 9/27/07, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > "Kelvie Wong" <kelvie@ieee.org> writes:
> >
> >> Egads, it's alive!
> >>
> >> I was in a subdirectory (most of my work is in that one subdirectory
> >> anyways :p), but running it on the top level did indeed work as
> >> expected.
> >>
> >> Thanks,
> >
> > Thanks for spotting a bug.  It claims to be subdirectory safe at
> > the top of the script but apparently it isn't.
> >
> > And I do not see a reason why it cannot be made subdirectory
> > safe.
>
> It _could_ be just the matter of doing this, although I cannot
> test it right now (at work and have no access to any of the
> backends).  Care to try it from a subdirectory and report
> failure or success?
>
> ---
>
>  git-mergetool.sh |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index a0e44f7..018db58 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -12,6 +12,7 @@ USAGE='[--tool=tool] [file to merge] ...'
>  SUBDIRECTORY_OK=Yes
>  . git-sh-setup
>  require_work_tree
> +cd_to_toplevel
>
>  # Returns true if the mode reflects a symlink
>  is_symlink () {
>

At least with emerge, this isn't so simple -- emacs tries to save it
as ${absolute_PWD}/${PWD_relative_to_toplevel}/$filename
(which of course doesn't exist yet).

In meld it works fine, however; haven't tried the other ones.
-- 
Kelvie

^ permalink raw reply

* Re: Mergetool generating blank files (1.5.3)
From: Kelvie Wong @ 2007-09-27 21:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <94ccbe710709271417h6349c807j6424c25175c26ea2@mail.gmail.com>

On 9/27/07, Kelvie Wong <kelvie@ieee.org> wrote:
> On 9/27/07, Junio C Hamano <gitster@pobox.com> wrote:
> > Junio C Hamano <gitster@pobox.com> writes:
> >
> > > "Kelvie Wong" <kelvie@ieee.org> writes:
> > >
> > >> Egads, it's alive!
> > >>
> > >> I was in a subdirectory (most of my work is in that one subdirectory
> > >> anyways :p), but running it on the top level did indeed work as
> > >> expected.
> > >>
> > >> Thanks,
> > >
> > > Thanks for spotting a bug.  It claims to be subdirectory safe at
> > > the top of the script but apparently it isn't.
> > >
> > > And I do not see a reason why it cannot be made subdirectory
> > > safe.
> >
> > It _could_ be just the matter of doing this, although I cannot
> > test it right now (at work and have no access to any of the
> > backends).  Care to try it from a subdirectory and report
> > failure or success?
> >
> > ---
> >
> >  git-mergetool.sh |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/git-mergetool.sh b/git-mergetool.sh
> > index a0e44f7..018db58 100755
> > --- a/git-mergetool.sh
> > +++ b/git-mergetool.sh
> > @@ -12,6 +12,7 @@ USAGE='[--tool=tool] [file to merge] ...'
> >  SUBDIRECTORY_OK=Yes
> >  . git-sh-setup
> >  require_work_tree
> > +cd_to_toplevel
> >
> >  # Returns true if the mode reflects a symlink
> >  is_symlink () {
> >
>
> At least with emerge, this isn't so simple -- emacs tries to save it
> as ${absolute_PWD}/${PWD_relative_to_toplevel}/$filename
> (which of course doesn't exist yet).
>
> In meld it works fine, however; haven't tried the other ones.
> --
> Kelvie
>

Hrm, on closer inspection, the old version of the mergetool script did this too.

It looks like the bug I mentioned does get fixed by that, and this is
a bug in emacs (or the way it's called).
-- 
Kelvie

^ permalink raw reply

* Re: Mergetool generating blank files (1.5.3)
From: Junio C Hamano @ 2007-09-27 21:33 UTC (permalink / raw)
  To: Kelvie Wong; +Cc: git
In-Reply-To: <94ccbe710709271422x5d1739c2g5da961c88a4336fe@mail.gmail.com>

"Kelvie Wong" <kelvie@ieee.org> writes:

>> > It _could_ be just the matter of doing this, although I cannot
>> > test it right now (at work and have no access to any of the
>> > backends).  Care to try it from a subdirectory and report
>> > failure or success?

Actually, it seems that mergetool can take paths arguments, and
they need to be adjusted, so here is a fixed patch.

---
 git-mergetool.sh |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index a0e44f7..56ec993 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -12,6 +12,8 @@ USAGE='[--tool=tool] [file to merge] ...'
 SUBDIRECTORY_OK=Yes
 . git-sh-setup
 require_work_tree
+directory_prefix=$(git rev-parse --show-prefix)
+cd_to_toplevel
 
 # Returns true if the mode reflects a symlink
 is_symlink () {
@@ -378,7 +380,7 @@ if test $# -eq 0 ; then
 else
 	while test $# -gt 0; do
 		printf "\n"
-		merge_file "$1"
+		merge_file "$directory_prefix$1"
 		shift
 	done
 fi

^ permalink raw reply related

* Re: Mergetool generating blank files (1.5.3)
From: Junio C Hamano @ 2007-09-27 21:41 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: git, Kelvie Wong
In-Reply-To: <7vmyv7sshv.fsf@gitster.siamese.dyndns.org>

When mergetool is run from a subdirectory, "ls-files -u" nicely
limits the output to conflicted files in that directory, but
we need to give the full path to cat-file plumbing to grab the
contents of stages.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * I earlier sent one with cd_to_toplevel but I think the
   approach in this patch is nicer.

 git-mergetool.sh |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index a0e44f7..3b1ec13 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -12,6 +12,7 @@ USAGE='[--tool=tool] [file to merge] ...'
 SUBDIRECTORY_OK=Yes
 . git-sh-setup
 require_work_tree
+prefix=$(git rev-parse --show-prefix)
 
 # Returns true if the mode reflects a symlink
 is_symlink () {
@@ -162,9 +163,9 @@ merge_file () {
     local_mode=`git ls-files -u -- "$path" | awk '{if ($3==2) print $1;}'`
     remote_mode=`git ls-files -u -- "$path" | awk '{if ($3==3) print $1;}'`
 
-    base_present   && git cat-file blob ":1:$path" > "$BASE" 2>/dev/null
-    local_present  && git cat-file blob ":2:$path" > "$LOCAL" 2>/dev/null
-    remote_present && git cat-file blob ":3:$path" > "$REMOTE" 2>/dev/null
+    base_present   && git cat-file blob ":1:$prefix$path" >"$BASE" 2>/dev/null
+    local_present  && git cat-file blob ":2:$prefix$path" >"$LOCAL" 2>/dev/null
+    remote_present && git cat-file blob ":3:$prefix$path" >"$REMOTE" 2>/dev/null
 
     if test -z "$local_mode" -o -z "$remote_mode"; then
 	echo "Deleted merge conflict for '$path':"

^ permalink raw reply related

* Re: [PATCH] quiltimport: Skip non-existent patches
From: Dan Nicholson @ 2007-09-27 21:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1wcju93a.fsf@gitster.siamese.dyndns.org>

On 9/27/07, Junio C Hamano <gitster@pobox.com> wrote:
> Dan Nicholson <dbn.lists@gmail.com> writes:
>
> > Dan Nicholson <dbn.lists <at> gmail.com> writes:
> >>
> >> When quiltimport encounters a non-existent patch in the series file,
> >> just skip to the next patch. This matches the behavior of quilt.
> >>
> >> Signed-off-by: Dan Nicholson <dbn.lists <at> gmail.com>
> >> ---
> >>  git-quiltimport.sh |    4 ++++
> >>  1 files changed, 4 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/git-quiltimport.sh b/git-quiltimport.sh
> >> index 74a54d5..880c81d 100755
> >> --- a/git-quiltimport.sh
> >> +++ b/git-quiltimport.sh
> >> @@ -71,6 +71,10 @@ commit=$(git rev-parse HEAD)
> >>
> >>  mkdir $tmp_dir || exit 2
> >>  for patch_name in $(grep -v '^#' < "$QUILT_PATCHES/series" ); do
> >> +    if ! [ -f "$QUILT_PATCHES/$patch_name" ] ; then
> >> +            echo "$patch_name doesn't exist. Skipping."
> >> +            continue
> >> +    fi
> >>      echo $patch_name
> >>      git mailinfo "$tmp_msg" "$tmp_patch" \
> >>              <"$QUILT_PATCHES/$patch_name" >"$tmp_info" || exit 3
> >
> >
> > I forgot to mention the rationale for this patch vs. what Junio sent. The issue
> > with Junio's patch is that the failure will occur before $tmp_patch is created
> > because the script tries to feed git-mailinfo a non-existent patch
> > ($patch_name). You'll only get past the mailinfo if $patch_name exists.
> >
> > The marker setting may still be useful in this context, though, to suppress the
> > "doesn't exist" message.
>
> Thanks.  I did not know what "marker" meant by the original
> context and assumed there is a file referred to by the series
> file but there is no patch in that file.  Instead it seems that
> a series file can contain something that is _not_ a file and
> that is called the marker, right?

I'm actually not a quilt user, but I tested out that patch on a repo
with a series containing a non-existent patch. I'm not sure what's
actually in Geerd's "marker", but I believe it's just random text.

When you run the command `quilt series', it just lists what's in the
series file (minus any comments). And when you run `quilt push' with a
non-existent patch, it says "Patch foo.patch does not exist; applied
empty patch"

So, I think the consistent thing to do is what's in my patch: just
skip the patch with a message to the user. Maybe the message can be
tailored to match quilt's output. Actually, it would be best to also
skip on empty files since quiltimport will bomb in that case as well.

--
Dan

^ permalink raw reply

* Re: backup or mirror a repository
From: Johannes Schindelin @ 2007-09-27 21:56 UTC (permalink / raw)
  To: Dan Farina; +Cc: git
In-Reply-To: <1190921742.2263.17.camel@Tenacity>

Hi,

On Thu, 27 Sep 2007, Dan Farina wrote:

> I have been poking around on IRC and gmane, but haven't yet seen a 
> solution to my problem:

I guess you're looking for "git remote add --mirror origin <url>".  AFAIK 
this is not in any released version yet, though.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] quiltimport: Skip non-existent patches
From: Junio C Hamano @ 2007-09-27 22:02 UTC (permalink / raw)
  To: Dan Nicholson; +Cc: git
In-Reply-To: <91705d080709271445k62f2867am99114eb0e98fc408@mail.gmail.com>

"Dan Nicholson" <dbn.lists@gmail.com> writes:

> When you run the command `quilt series', it just lists what's in the
> series file (minus any comments). And when you run `quilt push' with a
> non-existent patch, it says "Patch foo.patch does not exist; applied
> empty patch"
>
> So, I think the consistent thing to do is what's in my patch: just
> skip the patch with a message to the user. Maybe the message can be
> tailored to match quilt's output. Actually, it would be best to also
> skip on empty files since quiltimport will bomb in that case as well.

Thanks for your helpful explanation.  So perhaps we can do this
on top of yours to be safer and more consistent.

---

 git-quiltimport.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index 880c81d..627e023 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -79,8 +79,8 @@ for patch_name in $(grep -v '^#' < "$QUILT_PATCHES/series" ); do
 	git mailinfo "$tmp_msg" "$tmp_patch" \
 		<"$QUILT_PATCHES/$patch_name" >"$tmp_info" || exit 3
 	test -s "$tmp_patch" || {
-		echo "Patch is empty.  Was it split wrong?"
-		exit 1
+		echo "Patch is empty. Skipping."
+		continue
 	}
 
 	# Parse the author information

^ permalink raw reply related

* Re: [PATCH] quiltimport: Skip non-existent patches
From: Dan Nicholson @ 2007-09-27 22:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vejgjsr6k.fsf@gitster.siamese.dyndns.org>

On 9/27/07, Junio C Hamano <gitster@pobox.com> wrote:
> "Dan Nicholson" <dbn.lists@gmail.com> writes:
>
> > When you run the command `quilt series', it just lists what's in the
> > series file (minus any comments). And when you run `quilt push' with a
> > non-existent patch, it says "Patch foo.patch does not exist; applied
> > empty patch"
> >
> > So, I think the consistent thing to do is what's in my patch: just
> > skip the patch with a message to the user. Maybe the message can be
> > tailored to match quilt's output. Actually, it would be best to also
> > skip on empty files since quiltimport will bomb in that case as well.
>
> Thanks for your helpful explanation.  So perhaps we can do this
> on top of yours to be safer and more consistent.
>
> ---
>
>  git-quiltimport.sh |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-quiltimport.sh b/git-quiltimport.sh
> index 880c81d..627e023 100755
> --- a/git-quiltimport.sh
> +++ b/git-quiltimport.sh
> @@ -79,8 +79,8 @@ for patch_name in $(grep -v '^#' < "$QUILT_PATCHES/series" ); do
>         git mailinfo "$tmp_msg" "$tmp_patch" \
>                 <"$QUILT_PATCHES/$patch_name" >"$tmp_info" || exit 3
>         test -s "$tmp_patch" || {
> -               echo "Patch is empty.  Was it split wrong?"
> -               exit 1
> +               echo "Patch is empty. Skipping."
> +               continue
>         }
>
>         # Parse the author information

That's seems fine. IIUC, mailinfo will only create an empty patch if
there's no actual patch content in the original mail/patch. In that
case, you probably do want to skip and not bomb. I'd changed my patch
to do 'if ! [ -s "$patch" ]' to catch an empty file, but this is
probably better. Hmm, checking `quilt push' on a patch with no actual
patch bombs. Here's the output:

$ quilt push
Applying patch foo.patch
patch: **** Only garbage was found in the patch input.
Patch foo.patch does not apply (enforce with -f)
$ echo $?
1
$ cat patches/foo.patch
Here's info about an empty patch.

So, it might be better to leave the original behavior there to match quilt.

--
Dan

^ permalink raw reply

* Re: Mergetool generating blank files (1.5.3)
From: Theodore Tso @ 2007-09-27 22:23 UTC (permalink / raw)
  To: Pierre Habouzit, Russ Brown, Kelvie Wong, git
In-Reply-To: <20070927191125.GD12427@artemis.corp>

On Thu, Sep 27, 2007 at 09:11:25PM +0200, Pierre Habouzit wrote:
> > >   And as none of the other merge tool that are supported are able to
> > > either do 3way merges, or have a decent UI (that definitely seems to be
> > > exclusive features) I've given up on git-mergetool (and to be fair, it
> > > sucks, because it could be _sooo_ useful sometimes).
> > > 
> > 
> > What about meld? That does 3-way merge, and the UI is fine.
> 
>   Indeed, it seems that since the last time I tested it, it now does
> diff3 merging. I should reevaluate it :)

Pierre,

FYI, kdiff3, meld, xxdiff, opendiff (on MacOSX), and emerge all
support 3-way merge.

						- Ted

^ permalink raw reply

* Re: Mergetool generating blank files (1.5.3)
From: Kelvie Wong @ 2007-09-27 22:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Theodore Ts'o, git
In-Reply-To: <7vir5vss58.fsf@gitster.siamese.dyndns.org>

On 9/27/07, Junio C Hamano <gitster@pobox.com> wrote:
> When mergetool is run from a subdirectory, "ls-files -u" nicely
> limits the output to conflicted files in that directory, but
> we need to give the full path to cat-file plumbing to grab the
> contents of stages.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>
>  * I earlier sent one with cd_to_toplevel but I think the
>    approach in this patch is nicer.
>
>  git-mergetool.sh |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index a0e44f7..3b1ec13 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -12,6 +12,7 @@ USAGE='[--tool=tool] [file to merge] ...'
>  SUBDIRECTORY_OK=Yes
>  . git-sh-setup
>  require_work_tree
> +prefix=$(git rev-parse --show-prefix)
>
>  # Returns true if the mode reflects a symlink
>  is_symlink () {
> @@ -162,9 +163,9 @@ merge_file () {
>      local_mode=`git ls-files -u -- "$path" | awk '{if ($3==2) print $1;}'`
>      remote_mode=`git ls-files -u -- "$path" | awk '{if ($3==3) print $1;}'`
>
> -    base_present   && git cat-file blob ":1:$path" > "$BASE" 2>/dev/null
> -    local_present  && git cat-file blob ":2:$path" > "$LOCAL" 2>/dev/null
> -    remote_present && git cat-file blob ":3:$path" > "$REMOTE" 2>/dev/null
> +    base_present   && git cat-file blob ":1:$prefix$path" >"$BASE" 2>/dev/null
> +    local_present  && git cat-file blob ":2:$prefix$path" >"$LOCAL" 2>/dev/null
> +    remote_present && git cat-file blob ":3:$prefix$path" >"$REMOTE" 2>/dev/null
>
>      if test -z "$local_mode" -o -z "$remote_mode"; then
>         echo "Deleted merge conflict for '$path':"
>

--- a/git-mergetool       2007-09-24 09:08:23.000000000 -0700
+++ b/git-mergetool        2007-09-27 15:04:15.000000000 -0700
@@ -12,6 +12,7 @@
 SUBDIRECTORY_OK=Yes
 . git-sh-setup
 require_work_tree
+prefix=$(git rev-parse --show-prefix)

 # Returns true if the mode reflects a symlink
 is_symlink () {
@@ -162,9 +163,9 @@
     local_mode=`git ls-files -u -- "$path" | awk '{if ($3==2) print $1;}'`
     remote_mode=`git ls-files -u -- "$path" | awk '{if ($3==3) print $1;}'`

-    base_present   && git cat-file blob ":1:$path" > "$BASE" 2>/dev/null
-    local_present  && git cat-file blob ":2:$path" > "$LOCAL" 2>/dev/null
-    remote_present && git cat-file blob ":3:$path" > "$REMOTE" 2>/dev/null
+    base_present   && git cat-file blob ":1:$prefix$path" > "$BASE" 2>/dev/null
+    local_present  && git cat-file blob ":2:$prefix$path" > "$LOCAL"
2>/dev/null
+    remote_present && git cat-file blob ":3:$prefix$path" > "$REMOTE"
2>/dev/null

     if test -z "$local_mode" -o -z "$remote_mode"; then
        echo "Deleted merge conflict for '$path':"
@@ -251,7 +252,7 @@
            ;;
        emerge)
            if base_present ; then
-               emacs -f emerge-files-with-ancestor-command "$LOCAL"
"$REMOTE" "$BASE" "$path"
+               emacs -f emerge-files-with-ancestor-command "$LOCAL"
"$REMOTE" "$BASE" "$(basename "$path")"
            else
                emacs -f emerge-files-command "$LOCAL" "$REMOTE"
"$(basename "$path")"
            fi



Finally got it to work.  emacs (at least the version I'm using,
22.1.1) seems to set the $PWD via its first argument, that is,
$LOCAL's directory, and when it goes to save, it tries to save $path
on top of that.

The updated patch above would be just to use the basename, that is, if
it is certain that $LOCAL and $path will always reside in the same
directory -- and I believe, but am not certain, that this is the case.

Also, I am not sure if this is specific to my version of Emacs, so
perhaps some further testing is required.

-- 
Kelvie

^ permalink raw reply

* Re: Mergetool generating blank files (1.5.3)
From: Pierre Habouzit @ 2007-09-27 22:28 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Russ Brown, Kelvie Wong, git
In-Reply-To: <20070927222326.GB8688@thunk.org>

[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]

On Thu, Sep 27, 2007 at 10:23:26PM +0000, Theodore Tso wrote:
> On Thu, Sep 27, 2007 at 09:11:25PM +0200, Pierre Habouzit wrote:
> > > >   And as none of the other merge tool that are supported are able to
> > > > either do 3way merges, or have a decent UI (that definitely seems to be
> > > > exclusive features) I've given up on git-mergetool (and to be fair, it
> > > > sucks, because it could be _sooo_ useful sometimes).
> > > > 
> > > 
> > > What about meld? That does 3-way merge, and the UI is fine.
> > 
> >   Indeed, it seems that since the last time I tested it, it now does
> > diff3 merging. I should reevaluate it :)
> 
> Pierre,
> 
> FYI, kdiff3, meld, xxdiff, opendiff (on MacOSX), and emerge all
> support 3-way merge.

  I know, but:
  * kdiff3 often take decisions behind your back, and results in broken
    merges, so it's a no-go ;
  * xxdiff has (IMHO) a very bad and non-intuitive UI, I never get to
    make it work ;
  * I don't use macos (opendiff) ;
  * emerge is emacs right ? :)

  Though I gave meld another chance, and it works really better than it
used to, so I may give it a try :) Let's hope I won't be disappointed by
meld :)

-- 
·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: Mergetool generating blank files (1.5.3)
From: Theodore Tso @ 2007-09-27 22:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Kelvie Wong
In-Reply-To: <7vir5vss58.fsf@gitster.siamese.dyndns.org>

On Thu, Sep 27, 2007 at 02:41:23PM -0700, Junio C Hamano wrote:
> When mergetool is run from a subdirectory, "ls-files -u" nicely
> limits the output to conflicted files in that directory, but
> we need to give the full path to cat-file plumbing to grab the
> contents of stages.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Acked-by: "Theodore Ts'o" <tytso@mit.edu>

						- Ted

^ permalink raw reply

* Re: Mergetool generating blank files (1.5.3)
From: Theodore Tso @ 2007-09-27 22:52 UTC (permalink / raw)
  To: Kelvie Wong; +Cc: Junio C Hamano, git
In-Reply-To: <94ccbe710709271523s7e4c7a1dh53e34bd460c31d1f@mail.gmail.com>

On Thu, Sep 27, 2007 at 03:23:44PM -0700, Kelvie Wong wrote:
> Finally got it to work.  emacs (at least the version I'm using,
> 22.1.1) seems to set the $PWD via its first argument, that is,
> $LOCAL's directory, and when it goes to save, it tries to save $path
> on top of that.

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.

> The updated patch above would be just to use the basename, that is, if
> it is certain that $LOCAL and $path will always reside in the same
> directory -- and I believe, but am not certain, that this is the case.

> Also, I am not sure if this is specific to my version of Emacs, so
> perhaps some further testing is required.

Yep, I've checked both emacs21 and emacs23-snapshot, and they both use
write-file, so this seems to be a long-standing bug (and I would call
it that) in emerge.el.  So a patch like what you suggested is probably
going to be needed.

						- Ted

^ permalink raw reply

* Re: return void expressions in C (Was: [PATCH 3/6] Rework pretty_print_commit ...)
From: Linus Torvalds @ 2007-09-27 22:56 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Junio C Hamano, git
In-Reply-To: <20070910183256.GD32442@artemis.corp>



On Mon, 10 Sep 2007, Pierre Habouzit wrote:
>
>   Okay, I stand corrected, but as I'm curious, I've now the Truth about
> this, for those who still care at that point (as the patch has been
> fixed in the next round anyways ;p)

The

	void fn1(void);

	void fn2(void)
	{
		return fn1();
	}

thing is a gcc extension. It happens to be one we use in the kernel too, 
since it makes for nicer-looking source code in some circumstances, but 
it's definitely nonstandard.

		Linus

^ permalink raw reply

* Re: backup or mirror a repository
From: Dan Farina @ 2007-09-28  0:51 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0709272255360.28395@racer.site>

On Thu, 2007-09-27 at 22:56 +0100, Johannes Schindelin wrote:
> I guess you're looking for "git remote add --mirror origin <url>".  AFAIK 
> this is not in any released version yet, though.

I just tried a bleeding edge git with --mirror.

It's close, except (unless there is some other provision to take care of
this) remote branch deletes. (Which, after more gmane trolling, seemed
to be "git remote rm", which doesn't seem present just yet).

Of course, it is completely possible I'm using it incorrectly. In brief,
I did "git init" "git remote add --mirror backup <url>" "git
pull" (perhaps I should have used "git remote update"?) And the behavior
of this is more or less what I wanted, as far as sending branches.
Convenient removal of branches and propagating that removal still stumps
me. Is there a preferred way to do this yet?

Thanks,
fdr

^ permalink raw reply

* Re: backup or mirror a repository
From: Johannes Schindelin @ 2007-09-28  1:56 UTC (permalink / raw)
  To: Dan Farina; +Cc: git
In-Reply-To: <1190940704.2263.32.camel@Tenacity>

Hi,

On Thu, 27 Sep 2007, Dan Farina wrote:

> On Thu, 2007-09-27 at 22:56 +0100, Johannes Schindelin wrote:
> > I guess you're looking for "git remote add --mirror origin <url>".  
> > AFAIK this is not in any released version yet, though.
> 
> I just tried a bleeding edge git with --mirror.
> 
> It's close, except (unless there is some other provision to take care of 
> this) remote branch deletes. (Which, after more gmane trolling, seemed 
> to be "git remote rm", which doesn't seem present just yet).

git remote rm is about removing the remote nick, not a remote branch.

> Of course, it is completely possible I'm using it incorrectly. In brief, 
> I did "git init" "git remote add --mirror backup <url>" "git pull"

"git pull" without further parameters accesses the remote "origin", not 
"backup", and it merges _one_ branch into the current branch.

What you should have done is "git fetch backup".

Or use "origin" instead of "backup" right from the start, and then use 
"git fetch".

But no pull.

> (perhaps I should have used "git remote update"?) And the behavior of 
> this is more or less what I wanted, as far as sending branches. 
> Convenient removal of branches and propagating that removal still stumps 
> me. Is there a preferred way to do this yet?

You want to remove branches from the local repo which are no longer there 
on the remote side?

Then "git remote prune" is for you.  For details, please see the man page.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] git-gui: fix typo in lib/blame.tcl
From: Shawn O. Pearce @ 2007-09-28  2:14 UTC (permalink / raw)
  To: Michele Ballabio; +Cc: git
In-Reply-To: <200709271453.25772.barra_cuda@katamail.com>

Michele Ballabio <barra_cuda@katamail.com> wrote:
> diff --git a/lib/blame.tcl b/lib/blame.tcl
> index a911c3c..00ecf21 100644
> --- a/lib/blame.tcl
> +++ b/lib/blame.tcl
> @@ -905,7 +905,7 @@ method _open_tooltip {cur_w} {
>  		set author_time {}
>  		catch {set author_name $header($cmit,author)}
>  		catch {set summary     $header($cmit,summary)}
> -		catch {set author_time [foramt_date $header($cmit,author-time)]}
> +		catch {set author_time [format_date $header($cmit,author-time)]}

Shame on me.  Thanks for the fix.  :-)

-- 
Shawn.

^ permalink raw reply

* Re: backup or mirror a repository
From: Dan Farina @ 2007-09-28  2:37 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0709280253150.28395@racer.site>

On Fri, 2007-09-28 at 02:56 +0100, Johannes Schindelin wrote:
> Hi,
> 
> On Thu, 27 Sep 2007, Dan Farina wrote:

> git remote rm is about removing the remote nick, not a remote branch.

Ah, I thought for some reason there'd be a "remove" (since the other
operations have long names) and, much like "ls-remote" there'd be some
"rm-remote." An insane presupposition of mine from having too little
information.


> What you should have done is "git fetch backup".
> 
> Or use "origin" instead of "backup" right from the start, and then use 
> "git fetch".
> 
> But no pull.

I did use "pull backup" (my error for being sloppy and omitting it from
my mail) and it does work provided I do the dirty thing of doing a
subsequent reset --hard (I thought perhaps things were not yet
fully-there). Fetch does it perfectly, as you say, thanks!

> You want to remove branches from the local repo which are no longer there 
> on the remote side?
> 
> Then "git remote prune" is for you.  For details, please see the man page.
> 

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)

Thanks,
fdr

^ permalink raw reply

* [PATCH] alloc_ref(): allow for trailing NUL
From: Johannes Schindelin @ 2007-09-28  2:57 UTC (permalink / raw)
  To: gitster, git


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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 remote.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/remote.c b/remote.c
index e7d735b..7531c92 100644
--- a/remote.c
+++ b/remote.c
@@ -453,7 +453,7 @@ int remote_find_tracking(struct remote *remote, struct refspec *refspec)
 
 struct ref *alloc_ref(unsigned namelen)
 {
-	struct ref *ret = xmalloc(sizeof(struct ref) + namelen);
+	struct ref *ret = xmalloc(sizeof(struct ref) + namelen + 1);
 	memset(ret, 0, sizeof(struct ref) + namelen);
 	return ret;
 }
-- 
1.5.3.2.1102.g9487

^ permalink raw reply related

* Re: What's cooking in git.git (topics)
From: Daniel Barkalow @ 2007-09-28  3:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfy11yyxk.fsf@gitster.siamese.dyndns.org>

On Wed, 26 Sep 2007, Junio C Hamano wrote:

> * db/fetch-pack (Tue Sep 25 00:13:25 2007 -0400) 45 commits
>  + Prevent send-pack from segfaulting when a branch doesn't match
>  + Cleanup unnecessary break in remote.c
>  + Cleanup style nit of 'x == NULL' in remote.c
>  + Fix memory leaks when disconnecting transport instances
>  + Ensure builtin-fetch honors {fetch,transfer}.unpackLimit
>  + ...
> 
> Two issues known to me are:
> 
>  - "rsync" transport is not supported yet;
> 
>  - regresses "git pull <name>" using .git/remotes/<name>; does
>    not merge the first refspec when branch.<name>.merge is not
>    set.

Actually, I think it's only that it doesn't merge anything if the first 
refspec doesn't match the first fetched ref from the server. I'm running 
tests on a patch that should fix it, but I'm not sure how to write a test 
for this particular case; I think the fetch tests try to test this, but 
pass by chance.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* [PATCH] Merge non-first refs that match first refspec
From: Daniel Barkalow @ 2007-09-28  3:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

The code only looked at the first ref in the map of refs when looking
for matches for the first refspec in the case where there is not
per-branch configuration of ref to merge. This is often sufficient,
but not always. Make the logic clearer and test everything in the map.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
I ran all the usual tests with this, and it seems like it should fix a bug 
you saw, but I don't have the test case to make sure.

 builtin-fetch.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin-fetch.c b/builtin-fetch.c
index 2f639cc..47811c9 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -101,12 +101,13 @@ static struct ref *get_ref_map(struct transport *transport,
 				if (remote->fetch[i].dst &&
 				    remote->fetch[i].dst[0])
 					*autotags = 1;
-				if (!i && !has_merge && ref_map &&
-				    !strcmp(remote->fetch[0].src, ref_map->name))
-					ref_map->merge = 1;
 			}
 			if (has_merge)
 				add_merge_config(&ref_map, remote_refs, branch, &tail);
+			else
+				for (rm = ref_map; rm; rm = rm->next)
+					if (!strcmp(remote->fetch[0].src, rm->name))
+						rm->merge = 1;
 		} else {
 			ref_map = get_remote_ref(remote_refs, "HEAD");
 			ref_map->merge = 1;
-- 
1.5.3.2.1041.g2df10

^ permalink raw reply related

* Re: [PATCH] Merge non-first refs that match first refspec
From: Shawn O. Pearce @ 2007-09-28  4:15 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0709272351010.5926@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> wrote:
> The code only looked at the first ref in the map of refs when looking
> for matches for the first refspec in the case where there is not
> per-branch configuration of ref to merge. This is often sufficient,
> but not always. Make the logic clearer and test everything in the map.
> 
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---
> I ran all the usual tests with this, and it seems like it should fix a bug 
> you saw, but I don't have the test case to make sure.
> 
>  builtin-fetch.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/builtin-fetch.c b/builtin-fetch.c
> index 2f639cc..47811c9 100644
> --- a/builtin-fetch.c
> +++ b/builtin-fetch.c
> @@ -101,12 +101,13 @@ static struct ref *get_ref_map(struct transport *transport,
>  				if (remote->fetch[i].dst &&
>  				    remote->fetch[i].dst[0])
>  					*autotags = 1;
> -				if (!i && !has_merge && ref_map &&
> -				    !strcmp(remote->fetch[0].src, ref_map->name))
> -					ref_map->merge = 1;
>  			}
>  			if (has_merge)
>  				add_merge_config(&ref_map, remote_refs, branch, &tail);
> +			else
> +				for (rm = ref_map; rm; rm = rm->next)
> +					if (!strcmp(remote->fetch[0].src, rm->name))
> +						rm->merge = 1;
>  		} else {
>  			ref_map = get_remote_ref(remote_refs, "HEAD");
>  			ref_map->merge = 1;

Hmmph.  I'm not seeing how this fixes the bug.  But if it fixes it,
it fixes it.

My understanding of the old code was that it should do what Junio
was reporting as broken:

  - when i == 0 this is the first remote.$foo.fetch;
  - when has_merge == 0 we have no branch.$name.merge;
  - when ref_map != NULL we have at least one ref from this fetch spec;
  - when fetch[0].src == ref_map->name it wasn't a wildcard spec;

The if conditional above was ordered the way it is so we can skip
the more expensive operation (the strcmp) most of them time through
the loop iteration.

The only way I can see the old code was failing was if ref_map
as returned by get_fetch_map() pointed to more than one refspec.
But for that to be true then fetch[1].src must have been a wildcard,
in which case the strcmp() would fail.  So we should only ever
get one entry, it should be the first entry, and dammit it should
have matched.

How/why are we getting cases where fetch[0].src isn't in the first
entry in ref_map?  What are those other entries?  Are they possibly
going to also match fetch[0].src and cause more than one branch
to merge?

BTW, thanks for looking at this.  I didn't have time to get to it
this week and now I'm really unlikely to be able to do so until
after I get back from San Jose.  I have too many things crammed
into this next week. :-\

-- 
Shawn.

^ permalink raw reply

* Re: Mergetool generating blank files (1.5.3)
From: Kelvie Wong @ 2007-09-28  4:17 UTC (permalink / raw)
  To: Theodore Tso; +Cc: Junio C Hamano, git
In-Reply-To: <20070927225218.GD8688@thunk.org>

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 :(

-- 
Kelvie

^ permalink raw reply

* [EGIT PATCH] Won't append '/' to an empty repo relative path.
From: Jing Xue @ 2007-09-28  4:39 UTC (permalink / raw)
  To: git


When the repo relative path is empty, the extra "/" causes all subsequent
"startsWith" tests to fail.

Signed-off-by: Jing Xue <jingxue@digizenstudio.com>
---
 .../egit/ui/internal/actions/CommitAction.java     |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java
index 82cad58..7d638c7 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java
@@ -358,7 +358,11 @@ public class CommitAction implements IObjectActionDelegate {
 	}
 
 	private void includeList(IProject project, HashSet<String> added, ArrayList<IFile> category) {
-		String repoRelativePath = RepositoryMapping.getMapping(project).getRepoRelativePath(project) + "/";
+		String repoRelativePath = RepositoryMapping.getMapping(project).getRepoRelativePath(project);
+		if (repoRelativePath.length() > 0) { 
+			repoRelativePath += "/";
+		}
+
 		for (String filename : added) {
 			try {
 				if (!filename.startsWith(repoRelativePath))
-- 
1.5.3.2.116.ge36bb5

^ permalink raw reply related

* Re: [PATCH] Merge non-first refs that match first refspec
From: Daniel Barkalow @ 2007-09-28  4:40 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20070928041509.GU3099@spearce.org>

On Fri, 28 Sep 2007, Shawn O. Pearce wrote:

> My understanding of the old code was that it should do what Junio
> was reporting as broken:

Agreed; that's why I thought there must be something wrong with it.

>   - when i == 0 this is the first remote.$foo.fetch;
>   - when has_merge == 0 we have no branch.$name.merge;
>   - when ref_map != NULL we have at least one ref from this fetch spec;
>   - when fetch[0].src == ref_map->name it wasn't a wildcard spec;
> 
> The if conditional above was ordered the way it is so we can skip
> the more expensive operation (the strcmp) most of them time through
> the loop iteration.
> 
> The only way I can see the old code was failing was if ref_map
> as returned by get_fetch_map() pointed to more than one refspec.
> But for that to be true then fetch[1].src must have been a wildcard,
> in which case the strcmp() would fail.  So we should only ever
> get one entry, it should be the first entry, and dammit it should
> have matched.

That is the analysis that the original code is based on, yes. But it's not 
the easiest thing to follow, and I misunderstood it earlier this evening 
(prompted by the claim that it wasn't working, mostly). It's probably 
worth checking remote->fetch[0].pattern instead of the strcmp, anyway, 
since that's clearer and cheaper anyway.

> How/why are we getting cases where fetch[0].src isn't in the first
> entry in ref_map?  What are those other entries?  Are they possibly
> going to also match fetch[0].src and cause more than one branch
> to merge?

Beats me; Junio, what's your test case?

> BTW, thanks for looking at this.  I didn't have time to get to it
> this week and now I'm really unlikely to be able to do so until
> after I get back from San Jose.  I have too many things crammed
> into this next week. :-\

No problem. You fixed plenty of my other bugs in this code, and it's 
getting so close to done.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox