Git development
 help / color / mirror / Atom feed
* [PATCH next] t1505: remove debugging cruft
From: Thomas Rast @ 2009-01-24 22:23 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200901242322.02440.trast@student.ethz.ch>

Remove a call to git-log that I introduced for debugging and that
accidentally made it into d18ba22 (sha1_name: support @{-N} syntax in
get_sha1(), 2009-01-17).

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 t/t1505-rev-parse-last.sh |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh
index c745ec4..d709ecf 100755
--- a/t/t1505-rev-parse-last.sh
+++ b/t/t1505-rev-parse-last.sh
@@ -32,8 +32,6 @@ test_expect_success 'setup' '
 #
 # and 'side' should be the last branch
 
-git log --graph --all --pretty=oneline --decorate
-
 test_rev_equivalent () {
 
 	git rev-parse "$1" > expect &&
-- 
1.6.1.468.g15c0

^ permalink raw reply related

* Re: [VALGRIND PATCH for nth_last patch series] Fix invalid memory access
From: Thomas Rast @ 2009-01-24 22:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git, Johannes Sixt, Johan Herland
In-Reply-To: <200901211019.01493.trast@student.ethz.ch>

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

Thomas Rast wrote:
> Thanks for all the work you (both) are doing on this.  I hope to find
> the time to read the current state of the series tonight.

I was off by half a week :-(

The debugging 'git log --graph ...' that I forgot about is still in,
sorry.  I'll follow up with a small patch.  Other than that, I didn't
notice anything, though admittedly it wouldn't have been very hard to
sneak something past me.

By the way, you (Junio) remarked earlier in the thread that we could
forbid any use of 'git checkout -' except in literally that command
(i.e., no options or paths).  I'm midly opposed to it because I can
see myself saying 'git checkout -b sidebranch -' (and @{} is somewhat
awkward to type on Swiss keyboards).  Admittedly 'git checkout - --
file' looks rather confusing.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH] mergetool merge/skip/abort
From: Nanako Shiraishi @ 2009-01-24 21:45 UTC (permalink / raw)
  To: Caleb Cushing; +Cc: Junio C Hamano, Charles Bailey, git
In-Reply-To: <81bfc67a0901241036v6ca30c24q54487e118fd67c1c@mail.gmail.com>

Quoting Caleb Cushing <xenoterracide@gmail.com>:

>>  There are a few mergetool updates in flight from various authors.  How
>>  does your submission compare with others' in both form/presentation and
>>  clarity of logic (remember, I am not keeping track)?
>
> to be honest, a quick search of the past 2 months of patches didn't
> show me any patches that do the same thing as mine, so I'm not sure
> that comparing one feature to a different feature is good.

Junio never asked what your patch does. He didn't ask if it does
something similar to what other patches do, either.

Your 81bfc67a0901220617l22b5a8e4ma48bb069d67cae91@mail.gmail.com with
'Subject: Re: [PATCH] mergetool merge/skip/abort' that is sent to you and
'Cc: git@vger.kernel.org' starts its body with:

	From bf55fdd37f0fa4d0b3a10f43fa3d1815a6dbc6b3 Mon Sep 17 00:00:00 2001
	From: Caleb Cushing <xenoterracide@gmail.com>
	Date: Tue, 20 Jan 2009 11:33:30 -0500
	Subject: [PATCH] mergetool merge/skip/abort
	 add functionality to skip merging a file or abort from mergetool

	---
	 git-mergetool.sh |   20 ++++++++++++++++++--
	 1 files changed, 18 insertions(+), 2 deletions(-)

For comparison, 1232578668-2203-1-git-send-email-charles@hashpling.org from
Charles Bailey with 'Subject: [PATCH] mergetool: respect autocrlf by using
checkout-index', with 'Cc: Hannu Koivisto <azure@iki.fi>, Theodore Tso
<tytso@mit.edu>' starts its message body this way:

	Previously, git mergetool used cat-file which does not perform git to
	worktree conversion. This changes mergetool to use git checkout-index
	instead which means that the temporary files used for mergetool use the
	correct line endings for the platform.

	Signed-off-by: Charles Bailey <charles@hashpling.org>
	---
	 git-mergetool.sh     |   14 +++++++++++---
	 t/t7610-mergetool.sh |   15 +++++++++++++--
	 2 files changed, 24 insertions(+), 5 deletions(-)

Another example is 1232702093-24313-1-git-send-email-heipei@hackvalue.de
from Johannes Gilger with 'Subject: [PATCHv2] git mergetool: Don't repeat
merge tool candidates', sent to Junio, Theodore and the mailing list. Here
is its message body:

	git mergetool listed some candidates for mergetools twice, depending on
	the environment.

	Signed-off-by: Johannes Gilger <heipei@hackvalue.de>
	---
	The first patch had the fatal flaw that it listed nothing when DISPLAY 
	and EDITOR/VISUAL were unset, we fixed that.
	The order in which merge-candidates appear is still exactly the same, 
	only duplicates have been stripped. The check for KDE_FULL_SESSION was 
	removed since kdiff3 was added as long as DISPLAY was set and we weren't 
	running gnome.

	 git-mergetool.sh |   16 ++++++++--------
	 1 files changed, 8 insertions(+), 8 deletions(-)

Let's try to answer the first question Junio asked you together.
Can you spot the differences? How do they compare?

 1. You copy-and-pasted output from format-patch, and have the header
    part in the message body. Charles and Johannes have moved them to the
    Email header.

    Their messages are in the form the tool used for patch acceptance
    expects. Yours isn't, and forces Junio to manually edit your message
    before handling it.

 2. You have a two-line Subject: without any commit message. Both Charles
    and Johannes describe what their patches are about on the Subject
    succinctly in a single line, and they have what old behavior their
    patches change, and how their patches do so in their commit
    messages. They explained why it is good to apply their patches
    well. You didn't.

    Johannes Schindelin even pointed out this and the previous point when
    you sent your first version but you seem to have ignored him.

 3..You quoted other people's comments after the patch and explained that
    you addressed the issues, but didn't include them in your Cc list.
    Charles has Hanuu on his Cc list, and also Theodore (the original
    author) who knows the best about the tool. Johannes also sent his
    patch to people who gave him review comments.

    They made efforts to make sure that their patches are seen by people
    who helped refine thier patches and/or by people who knows the script
    that you are modifying well. You didn't.

 4. You didn't sign your patch.

    Please see Documentation/SubmittingPatches.

About the second question from Junio on the contents of the patch, I can
guess some comments you may receive from him when he reads your patch,
based on review comments I received from him on another shell script
recently.

	diff --git a/git-mergetool.sh b/git-mergetool.sh
	index 00e1337..bd5711e 100755
	--- a/git-mergetool.sh
	+++ b/git-mergetool.sh
	@@ -177,8 +177,24 @@ merge_file () {
	     describe_file "$local_mode" "local" "$LOCAL"
	     describe_file "$remote_mode" "remote" "$REMOTE"
	     if "$prompt" = true; then
	-       printf "Hit return to start merge resolution tool (%s): " "$merge_tool"
	-       read ans
	+        while true; do
	+            printf "Use (m)erge file or (s)kip file, or (a)bort? (%s): " \
	+            "$merge_tool"
	+            read ans
	+            case "$ans" in
	+                [mM]*|"")
	+                    break
	+                ;;
	+                [sS]*)
	+                    cleanup_temp_files
	+                    return 0
	+                ;;
	+                [aA]*)
	+                    cleanup_temp_files
	+                    exit 0
	+                ;;
	+            esac
	+        done
	     fi

	     case "$merge_tool" in

 1. Your printf message is funny. You either

      (1) Use $merge_tool to merge file, or
      (2) Skip file, or
      (3) Abort.

    but your message makes it look like:

      (1) Use $merge_tool to Merge file, or
      (2) Use $merge_tool to Skip file, or
      (3) Use $merge_tool to Abort.

 2. patterns in case command start at the same column as case and esac,
    and ";;" is at the same column as any other commands.

	case "$ans" in
	[mM]*|"")
		break
		;;
	[Ss]*)
		...
	esac

For what it's worth, I like what your patch does. I use mergetool from
time to time and I can imagine that this new feature will be useful.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: CR codes from git commands
From: Johannes Schindelin @ 2009-01-24 21:14 UTC (permalink / raw)
  To: Brent Goodrick; +Cc: Daniel Barkalow, Mike Ralphson, Junio C Hamano, git
In-Reply-To: <18811.32772.728276.923430@hungover.brentg.com>

Hi,

On Sat, 24 Jan 2009, Brent Goodrick wrote:

> Note that I'm coming from a CVS and Perforce user background but am 
> still new to git usage. How do I "take a look" at "7d1864c"?

Do this in a checkout of git.git:

$ git show 7d1864c

Alternatively, you can follow this URL:

	http://repo.or.cz/w/git.git?a=commitdiff;h=7d1864c

Ciao,
Dscho

^ permalink raw reply

* Re: Heads up: major rebase -i -p rework coming up
From: Johannes Schindelin @ 2009-01-24 21:09 UTC (permalink / raw)
  To: Junio C Hamano, Stephan Beyer
  Cc: git, Stephen Haberman, spearce, Thomas Rast,
	Björn Steinbrink
In-Reply-To: <alpine.DEB.1.00.0901242156320.14855@racer>

Hi,

On Sat, 24 Jan 2009, Johannes Schindelin wrote:

> > - Why do you need "merge D' was E"?  Shouldn't "pick E" be able to 
> >   notice that E is a merge and decompose it into "merge D' was E"  
> >   internally?
> > 
> >   This one I am somewhat complaining, unless your answer is "because 
> >   this way the user could drop some parents from the merge in the 
> >   editor".
> 
> Not only that; the user could use this to fix mismerges, i.e. by 
> replacing a SHA-1 with the SHA-1 (or indeed, a short name, unless it is 
> "was") of the branch that she _actually_ wanted to merge with.
> 
> >   And if your answer is that, then my next question will be "if that 
> >   is the case, can the user be expected to easily find out which 
> >   commit each parent SHA-1 refers to, without having more hint on the 
> >   'merge' insn line?"
> 
> Nope.
> 
> In most cases, however, that should be plenty enough:
> 
> 	merge 9383af1' was f39d50a Merge branch 'mh/unify-color' into next
> 
> The user does not have to guess much what 9383af1 might refer to.

Heh, I think it is much easier than I thought:  How about this?

 	merge 9383af1' was f39d50a Merge branch 'mh/unify-color' into next
	#   \ 9383af1 Revert previous two commits

Obviously, for octopodes, there would be multiple "#   \ <SHA-1> <oneline>" 
lines...

Ciao,
Dscho

^ permalink raw reply

* Re: Heads up: major rebase -i -p rework coming up
From: Johannes Schindelin @ 2009-01-24 21:04 UTC (permalink / raw)
  To: Junio C Hamano, Stephan Beyer
  Cc: git, Stephen Haberman, spearce, Thomas Rast,
	Björn Steinbrink
In-Reply-To: <7vpricmoda.fsf@gitster.siamese.dyndns.org>

Hi,

On Sat, 24 Jan 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > 	pick A
> > 	pick C
> > 	pick D
> > 	goto A'
> > 	pick B
> > 	merge D' was E
> >
> > This should lead to a much more intuitive user experience.
> >
> > I am very sorry if somebody actually scripted rebase -i -p (by setting 
> > GIT_EDITOR with a script), but I am very certain that this cleanup is 
> > absolutely necessary to make rebase -i -p useful.
> 
> Three questions.
> 
> - An obvious one first.  How does this relate to the sequencer project 
>   (that seems to have gone somewhat dark?)

As far as I can see, Stephan can keep the "mark" command he cherishes so 
much, and we can still use thise syntax for rebase -i -p.

> - What's with the apostrophe?  I seem to remember that you argued it 
>   would be enough to make "A" stand for the original when it is used for 
>   the first time and the second and later use can stand for the result 
>   of the last use (e.g. the "goto A'" above can be simply spelled as 
>   "goto A"), when I suggested to use "mark" in a way similar to how 
>   fast-import language uses it during the sequencer discussion?
> 
>   I am not complaining; I am just being curious why the sudden change of 
>   heart.

Very easy explanation.  I got convinced by your arguments.  Even if I 
could imagine that I never use the thing without apostrophe, it is good to 
have an obvious indicator that this is not necessarily the original 
commit.

> - Why do you need "merge D' was E"?  Shouldn't "pick E" be able to 
>   notice that E is a merge and decompose it into "merge D' was E" 
>   internally?
> 
>   This one I am somewhat complaining, unless your answer is "because 
>   this way the user could drop some parents from the merge in the 
>   editor".

Not only that; the user could use this to fix mismerges, i.e. by replacing 
a SHA-1 with the SHA-1 (or indeed, a short name, unless it is "was") of 
the branch that she _actually_ wanted to merge with.

>   And if your answer is that, then my next question will be "if that is 
>   the case, can the user be expected to easily find out which commit 
>   each parent SHA-1 refers to, without having more hint on the 'merge' 
>   insn line?"

Nope.

In most cases, however, that should be plenty enough:

	merge 9383af1' was f39d50a Merge branch 'mh/unify-color' into next

The user does not have to guess much what 9383af1 might refer to.

In case of octopodes, or when the commit message was changed, the user has 
to open another command line and look for herself, though.

Ciao,
Dscho

^ permalink raw reply

* Re: CR codes from git commands
From: Brent Goodrick @ 2009-01-24 20:54 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Brent Goodrick, Daniel Barkalow, Mike Ralphson, Junio C Hamano,
	git
In-Reply-To: <alpine.DEB.1.00.0901231747340.21467@intel-tinevez-2-302>


Junio C Hamano writes:
 > I do not think so.  --no-progress should imply --no-cr ;-)
 > 
 > I do not think it makes much sense to pollute your non-terminal with 100
 > lines of 1%,2%,3%,...100% if it cannot sensibly do carriage-returns.  It
 > may be another knob to tweak, but it's a kind of thing you implement
 > because you could, not because it makes sense.  I would be mildly against
 > no-cr.

Good point. I'll drop the --no-cr as redundant.

Johannes Schindelin writes:
 > Hi,
 > 
 > On Fri, 23 Jan 2009, Brent Goodrick wrote:
 > 
 > >  - Bare minimum: Add a new --no-cr option
 > 
 > I do not see any value of this over "--progress | tr '\r' '\n'".  (The 
 > --progress option being the natural counterpart to --no-progress, 
 > _forcing_ the display of the progress.)

Agreed. Both --progress and --no-progress are the only options to be
implemented for this.  

 > Just have a 
 > look at 7d1864c(Introduce is_bare_repository() and core.bare configuration 
 > variable).

Note that I'm coming from a CVS and Perforce user background but am
still new to git usage. How do I "take a look" at "7d1864c"?

I will take a closer look at the list of things you explained in your
"Basically, you'll have to" list.

While I'm at it, what is the standard procedure for submitting git
patches for review once I've cooked up and validated it on my end? I'm
guessing posting the patch into this mailing list is part of the
answer to that question.

Thanks,
Brent

^ permalink raw reply

* Re: [PATCH 1/2] handle color.ui at a central place
From: Jeff King @ 2009-01-24 20:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: markus.heidelberg, René Scharfe, git
In-Reply-To: <7vvds4movp.fsf@gitster.siamese.dyndns.org>

On Sat, Jan 24, 2009 at 12:26:50PM -0800, Junio C Hamano wrote:

> > Sadly, this is an area that is not covered very well in the tests
> > (partially, I think, because it is "just" output which we tend to
> > neglect, and partially because the isatty() stuff is hard to test with
> > our harness). So I don't think it's _entirely_ Markus' fault.
> 
> Oh, don't get me wrong.  I am not interested in finding whose fault it
> was.  I was just stating the fact that one person not finding a breakage
> does not mean much as an assurance.

OK. I was just trying to encourage Markus to keep trying. ;)

> +test_expect_success 'format-patch is colorless even with color.ui = auto' '
> +	git config color.ui auto &&
> +	TERM=ansi git format-patch -1 >/dev/tty &&
> +	grep "^+5$" 0001-foo.patch
> +'
> +

Actually, could this not just be "format-patch is colorless even with
color.ui = always"? I.e., shouldn't format-patch _always_ not have
color, no matter what (and we assume that "always" is a superset of
"auto"). And then it is also much easier to test.

Of course that doesn't fix the fact that it would _also_ be nice to test
"auto" functionality in general. Here you use the fact that format-patch
sends output to a file. But I don't think there is a good way to test
that "git log --color=auto" works as expected.

-Peff

^ permalink raw reply

* Re: Heads up: major rebase -i -p rework coming up
From: Junio C Hamano @ 2009-01-24 20:37 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: git, Stephen Haberman, spearce, Thomas Rast,
	Björn Steinbrink
In-Reply-To: <alpine.DEB.1.00.0901242056070.14855@racer>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> 	pick A
> 	pick C
> 	pick D
> 	goto A'
> 	pick B
> 	merge D' was E
>
> This should lead to a much more intuitive user experience.
>
> I am very sorry if somebody actually scripted rebase -i -p (by setting 
> GIT_EDITOR with a script), but I am very certain that this cleanup is 
> absolutely necessary to make rebase -i -p useful.

Three questions.

- An obvious one first.  How does this relate to the sequencer project (that
  seems to have gone somewhat dark?)

- What's with the apostrophe?  I seem to remember that you argued it would
  be enough to make "A" stand for the original when it is used for the
  first time and the second and later use can stand for the result of the
  last use (e.g. the "goto A'" above can be simply spelled as "goto A"),
  when I suggested to use "mark" in a way similar to how fast-import
  language uses it during the sequencer discussion?

  I am not complaining; I am just being curious why the sudden change of
  heart.

- Why do you need "merge D' was E"?  Shouldn't "pick E" be able to notice
  that E is a merge and decompose it into "merge D' was E" internally?

  This one I am somewhat complaining, unless your answer is "because this
  way the user could drop some parents from the merge in the editor".

  And if your answer is that, then my next question will be "if that is
  the case, can the user be expected to easily find out which commit each
  parent SHA-1 refers to, without having more hint on the 'merge' insn
  line?"

^ permalink raw reply

* Re: [PATCH 1/2] handle color.ui at a central place
From: Junio C Hamano @ 2009-01-24 20:26 UTC (permalink / raw)
  To: Jeff King; +Cc: markus.heidelberg, René Scharfe, git
In-Reply-To: <20090124191700.GA17935@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Sat, Jan 24, 2009 at 10:36:24AM -0800, Junio C Hamano wrote:
> ...
>> You did not find the breakage in format-patch either to begin with; so
>> your not finding does not give us much confidence that there is no other
>> breakage, does it?
>> 
>> Grumble...
>
> Sadly, this is an area that is not covered very well in the tests
> (partially, I think, because it is "just" output which we tend to
> neglect, and partially because the isatty() stuff is hard to test with
> our harness). So I don't think it's _entirely_ Markus' fault.

Oh, don't get me wrong.  I am not interested in finding whose fault it
was.  I was just stating the fact that one person not finding a breakage
does not mean much as an assurance.

It is actually not trivial to test this breakage in our test suite.
Before committing 9383af1 (Revert previous two commits, 2009-01-23), I
spent about 20 minutes trying to come up with a test to expose the
breakage in an acceptable way.  A test that assumes that it is run with a
controlling terminal is relatively easy to write, but I couldn't come up
with a test that would have triggered even when the tests were run without
a tty (for gory details, see git_config_colorbool() and how stdout_is_tty
is used).

Here is the "relatively easy" but an unacceptable one.

diff --git c/t/t4014-format-patch.sh w/t/t4014-format-patch.sh
index 9d99dc2..609946a 100755
--- c/t/t4014-format-patch.sh
+++ w/t/t4014-format-patch.sh
@@ -255,4 +255,10 @@ test_expect_success 'format-patch respects -U' '
 
 '
 
+test_expect_success 'format-patch is colorless even with color.ui = auto' '
+	git config color.ui auto &&
+	TERM=ansi git format-patch -1 >/dev/tty &&
+	grep "^+5$" 0001-foo.patch
+'
+
 test_done

Points that makes the above patch unacceptable are:

 (1) It hardcodes 0001-foo.patch.  You could try doing these:

     (1.1) patchname=$( ... git format-patch -1) && grep ... <"$patchname"
     (1.2) git format-patch -1 --stdout >patchfile && grep ... <patchfile

     but they won't work, because "color.ui = auto" will not color unless
     the standard output is a tty, and TERM is better than "dumb".

 (2) It would not trigger if /dev/tty cannot be opened for writing.

     

^ permalink raw reply related

* Heads up: major rebase -i -p rework coming up
From: Johannes Schindelin @ 2009-01-24 20:25 UTC (permalink / raw)
  To: git, Stephen Haberman, spearce, Thomas Rast,
	Björn Steinbrink

Hi all,

as probably everybody agrees, the code to preserve merges is a big mess 
right now.

Worse, the whole concept of "pick <merge-sha1>" just does not fly well.

So I started a _major_ cleanup, which happens to reduce the code very 
nicely so far.

It will take a few days to flesh out, I guess, but these are the major 
ideas of my work:

- pick $sha1

	will only work on non-merges in the future

- merge $sha1 [$sha1...] was $sha1 "Merge ..."

	will merge the given list of commits into the current HEAD, for 
	the user's reference and to keep up-to-date what was rewritten, 
	the original merge is shown after the keyword "was" (which is not 
	a valid SHA-1, luckily)

- goto $sha1

	will reset the HEAD to the given commit

- $sha1'

	for merge and goto, if a $sha1 ends in a single quote, the 
	rewritten commit is substituted (if there is one)

Example:

A - B - - - E 
  \       /
    C - D

could yield this TODO script:

	pick A
	pick C
	pick D
	goto A'
	pick B
	merge D' was E

This should lead to a much more intuitive user experience.

I am very sorry if somebody actually scripted rebase -i -p (by setting 
GIT_EDITOR with a script), but I am very certain that this cleanup is 
absolutely necessary to make rebase -i -p useful.

As always, I am thankful for suggestions to make this even more useful, or 
even easier to operate.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH v2] Makefile: Use libc strlcpy on OSX >= 10.2
From: Benjamin Kramer @ 2009-01-24 20:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8wp0o5il.fsf@gitster.siamese.dyndns.org>

On Sat, Jan 24, 2009 at 20:42, Junio C Hamano <gitster@pobox.com> wrote:
>
> This existing one says "If 'uname -r' output does not begin with two
> characters '9.' (nine and dot), then set OLD_ICONV".
>
> I am guessing that 'uname -r' says 9.X in "Darwin 9.X" (which is OSX
> 10.5), and existing conditional says that versions before 9.X needs
> OLD_ICONV but later ones do not need it.

Yeah, that's because versions before 10.5 aka darwin 9 aka "Leopard" have a
broken iconv implementation.

> Does 1.X stand for OSX 10.2?  A quick googling finds a handful pages that
> say that OSX 10.2 = Darwin 6.x and OSX 10.3 = Darwin 7.x, and I am not
> sure where you are getting that "begins with one-and-dot" from....

I've got my information from [1]. It says:

darwin 1.3 => OSX 10.0
darwin 1.4 => OSX 10.1
darwin 6 => OSX 10.2
darwin 7 => OSX 10.3
etc.

I don't know why they chose this weird versioning scheme, but wait, it gets
even crazier

darwin 1.4.1 => OSX 10.1
darwin 5.1 => OSX 10.1.1

So my patch checks only for 10.1 but not for 10.1.1 *sigh*

-     NO_STRLCPY = YesPlease
+     ifeq ($(shell expr "$(uname_R)" : '[015]\.'),2)
+             NO_STRLCPY = YesPlease
+     endif

Should get all darwin versions before 6.0 in which strlcpy(3) was
introduced. Thanks for your patience ;)

---------------
[1] http://www.opensource.apple.com/darwinsource/

^ permalink raw reply

* Re: [RFC/PATCH v3 3/3] archive.c: add basic support for submodules
From: Lars Hjemli @ 2009-01-24 20:02 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Junio C Hamano, René Scharfe
In-Reply-To: <alpine.DEB.1.00.0901242049580.14855@racer>

On Sat, Jan 24, 2009 at 20:52, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Sat, 24 Jan 2009, Lars Hjemli wrote:
>
>> $ git archive --submodules <tree-ish>: Create an archive which
>> includes the trees of all gitlink entries in <tree-ish>, fail unless
>> all the required objects are available.
>>
>> $ git archive --submodules=<group>: Same as above, but only traverse
>> submodules in the specified group (as defined in $GIT_CONFIG).
>
> How about having the former with --submodules='*' and let --submodules
> without argument include those submodules that are checked out (none in a
> bare repository)?

Yeah, that might make more sense (since you'd normally not have access
to the content of non-checked out submodules). I'm also considering
something like --submodules[=(all|checkedout|[group:]<name>)], i.e.
the 'group:'-part could be optional as long as <name> is unambiguous.

--
larsh

^ permalink raw reply

* Re: diff settings
From: Keith Cascio @ 2009-01-24 20:01 UTC (permalink / raw)
  To: Teemu Likonen; +Cc: git
In-Reply-To: <87ocxwv6qg.fsf@iki.fi>

> It seems that the answer is "no", but aliases are a work-around:
> 
>     git config --global alias.dff "diff -w"
> 
> Then "git dff" is your new "git diff -w". :-)

That feature is gonna come in handy.  It doesn't 100% fulfill my needs here but 
I like it!  Thanks, I did not know about alias.*

^ permalink raw reply

* Re: [RFC/PATCH v3 3/3] archive.c: add basic support for submodules
From: Johannes Schindelin @ 2009-01-24 19:52 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: git, Junio C Hamano, René Scharfe
In-Reply-To: <8c5c35580901241126q2da83f50m1472ed017b92c982@mail.gmail.com>

Hi,

On Sat, 24 Jan 2009, Lars Hjemli wrote:

> $ git archive --submodules <tree-ish>: Create an archive which
> includes the trees of all gitlink entries in <tree-ish>, fail unless
> all the required objects are available.
> 
> $ git archive --submodules=<group>: Same as above, but only traverse
> submodules in the specified group (as defined in $GIT_CONFIG).

How about having the former with --submodules='*' and let --submodules 
without argument include those submodules that are checked out (none in a 
bare repository)?

Thanks,
Dscho

^ permalink raw reply

* Re: [PATCH v2] Makefile: Use libc strlcpy on OSX >= 10.2
From: Junio C Hamano @ 2009-01-24 19:42 UTC (permalink / raw)
  To: Benjamin Kramer; +Cc: git
In-Reply-To: <7f978c810901241101h227c5437p88ca7c9cc54abc9a@mail.gmail.com>

Benjamin Kramer <benny.kra@googlemail.com> writes:

> OSX supports strlcpy(3) since 10.2 so we don't need
> to use our own. This revised also patch checks the darwin
> version. 10.0 is darwin v1.3; 10.1 is darwin v1.4.

What does the output from "uname -r" and "uname -s" look like on your box
(and older OSX boxes)?

> @@ -643,7 +643,9 @@ ifeq ($(uname_S),Darwin)
>  	ifneq ($(shell expr "$(uname_R)" : '9\.'),2)
>  		OLD_ICONV = UnfortunatelyYes
>  	endif

This existing one says "If 'uname -r' output does not begin with two
characters '9.' (nine and dot), then set OLD_ICONV".

> -	NO_STRLCPY = YesPlease
> +	ifeq ($(shell expr "$(uname_R)" : '1\.'),2)

And this new one says "If 'uname -r' output begins with two characters
'1.' (one and dot), then set NO_STRLCPY".

> +		NO_STRLCPY = YesPlease
> +	endif

I am guessing that 'uname -r' says 9.X in "Darwin 9.X" (which is OSX
10.5), and existing conditional says that versions before 9.X needs
OLD_ICONV but later ones do not need it.

Does 1.X stand for OSX 10.2?  A quick googling finds a handful pages that
say that OSX 10.2 = Darwin 6.x and OSX 10.3 = Darwin 7.x, and I am not
sure where you are getting that "begins with one-and-dot" from....

>  	NO_MEMMEM = YesPlease
>  	THREADED_DELTA_SEARCH = YesPlease
>  endif
> -- 
> 1.6.1.285.g3454

^ permalink raw reply

* Re: read-only working copies using links
From: Jeff King @ 2009-01-24 19:35 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Chad Dombrova, Tim 'Mithro' Ansell, git
In-Reply-To: <bd6139dc0901241043h7087316bgcbcb5d9085d603ef@mail.gmail.com>

On Sat, Jan 24, 2009 at 07:43:20PM +0100, Sverre Rabbelier wrote:

> On Sat, Jan 24, 2009 at 19:39, Chad Dombrova <chadrik@gmail.com> wrote:
> > what's the case against it, other than the obvious, that it will take more
> > work?
> 
> Good question, I think it was mostly that, someone has to implement it
> (possibly as part of packv4). Backwards compatibility is of course
> always an concern, but I'm not too familiar with the subject, perhaps
> other people on the list (or even those were at the gittogether) can
> comment?

If I understand his proposal correctly, such objects must _not_ be part
of a pack. The whole idea is splitting them _more_, not less.

-Peff

^ permalink raw reply

* Re: read-only working copies using links
From: Jeff King @ 2009-01-24 19:34 UTC (permalink / raw)
  To: Chad Dombrova; +Cc: Sverre Rabbelier, Tim 'Mithro' Ansell, git
In-Reply-To: <AE9781AD-0B63-4AEE-9B62-20F9720B5DC4@gmail.com>

On Sat, Jan 24, 2009 at 10:39:46AM -0800, Chad Dombrova wrote:

>> I think Tim Ansell (cced) was talking about this at the gittogether
>> (storing the metadata seperately), as it would benefit sparse/narrow
>> checkout, another advantage supporting his case?
>
> what's the case against it, other than the obvious, that it will take  
> more work?

I'm not sure this is actually the same as Tim's proposal. Tim wanted to
store the commit and tree information separately from the blob
information (since his use case was that blobs are enormous, but the
rest is reasonable).

AIUI, Chad's proposal is about storing the actual blob data itself
separate from the blob object's metadata (i.e., its object type and
length headers). Which means that the normal loose object format is not
acceptable, and you would end up with something like (for example):

  .git/objects/pack/pack-full-of-your-regular-stuff.{pack,idx}
  .git/objects/[0-9a-f]{2}/[0-9a-f]{38}/header
  .git/objects/[0-9a-f]{2}/[0-9a-f]{38}/data

or something similar. Then you could hardlink directly to the 'data'
portion. So you would need:

  - to teach everything that ever looks for loose objects how to read
    this new format. In theory, it's all nicely encapsulated in
    sha1_file.c

  - to teach checkout routines to hardlink such a case instead of
    copying the file

The obvious downsides that I can think of are:

  - it has the potential to make object reading, which is a core part of
    git (read: very performance- and correctness- sensitive) a lot more
    complex. But maybe the implementation would not be that painful;
    somebody would have to look very closely to see.

  - it interacts badly with smudge/clean filters and crlf conversion.
    In those cases you can't hardlink. If you treat this like an
    optimization, though, it's not so bad: we only do the optimization
    when we _can_, and fall back to regular checkout if those other
    options are in effect.

  - it's somewhat dangerous to your repository's health. Git's model is
    that object files are immutable (since they are, after all, named
    after their contents). But now you are linking them into your
    working tree, which makes them susceptible to some third party tool
    munging them. So yes, most tools will probably behave, but any tool
    that misbehaves will actually corrupt your repository.

-Peff

^ permalink raw reply

* Re: diff settings
From: Teemu Likonen @ 2009-01-24 19:33 UTC (permalink / raw)
  To: Keith Cascio; +Cc: git
In-Reply-To: <alpine.GSO.2.00.0901240936080.23073@kiwi.cs.ucla.edu>

Keith Cascio (2009-01-24 09:38 -0800) wrote:

> How do I configure my local git so that diff always obeys a particular
> option, e.g. "-w", without needing to type it on the command line each
> time?

It seems that the answer is "no", but aliases are a work-around:

    git config --global alias.dff "diff -w"

Then "git dff" is your new "git diff -w". :-)

^ permalink raw reply

* Re: [RFC/PATCH v3 3/3] archive.c: add basic support for submodules
From: Lars Hjemli @ 2009-01-24 19:26 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Junio C Hamano, René Scharfe
In-Reply-To: <alpine.DEB.1.00.0901241443270.13232@racer>

On Sat, Jan 24, 2009 at 14:51, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Now, there is still a problem when your submodule is missing the objects
> for the commit your superproject is referring to.
>
> IMO that is a serious issue, as it just asks for confused users.

This made me finally understand your concern (sorry for being slow):
you want the command to behave in a predictable/consistent way while
my implementation would end up making an archive with basically random
content.


>> > - presence of a specific commit in the supermodule is a _lousy_
>> >   indicator that the user wants to include that submodule in the
>> >   archive.
>>
>> This is the issue I tried to address with my
>> `--submodules=[a|c|r][g:<name>]` proposal in the commit message for
>> this patch.
>
> Nope, doing this "in the future" does not please me one bit.
>
> Besides, I find the semantics, uhm, "interesting".  (The other word would
> be "unintuitive".  Why do you have to be so cryptic that I have to read
> the proposal to understand what the heck "c" is about?)

I thought it would be nifty to be able to combine different flags
which would affect the behaviour/semantics of the command, but given
the comments from you and Junio, I think I'll end up with something
like this:

$ git archive --submodules <tree-ish>: Create an archive which
includes the trees of all gitlink entries in <tree-ish>, fail unless
all the required objects are available.

$ git archive --submodules=<group>: Same as above, but only traverse
submodules in the specified group (as defined in $GIT_CONFIG).

--
larsh

^ permalink raw reply

* Re: diff settings
From: Jeff King @ 2009-01-24 19:22 UTC (permalink / raw)
  To: Keith Cascio; +Cc: git
In-Reply-To: <alpine.GSO.2.00.0901240936080.23073@kiwi.cs.ucla.edu>

On Sat, Jan 24, 2009 at 09:38:18AM -0800, Keith Cascio wrote:

> How do I configure my local git so that diff always obeys a particular
> option, e.g. "-w", without needing to type it on the command line each
> time?

AFAIK, there is not a way to do it in the general case (though there are
specific config options for things like rename support). When I
refactored the gitattributes diff=$foo code a few months back, I tried
to keep in mind the possibility of a "diff options" field. But I never
actually implemented anything, as I don't personally ever use diff
options in such a way.

If you're interested, it would probably not be too hard to implement
(see userdiff.[ch], and the external and textconv code in diff.c for a
start).

-Peff

^ permalink raw reply

* Re: [PATCH 1/2] handle color.ui at a central place
From: Jeff King @ 2009-01-24 19:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: markus.heidelberg, René Scharfe, git
In-Reply-To: <7vk58ko8k7.fsf@gitster.siamese.dyndns.org>

On Sat, Jan 24, 2009 at 10:36:24AM -0800, Junio C Hamano wrote:

> Why should format-patch need to even worry about protecting itself from
> "color.ui" to begin with?

Agreed. In the "should I use color" function I proposed, there should be
a big fat "are_we_a_porcelain_that_will_allow_any_color_at_all" flag
at the top, which will make it totally clear how to make sure color is
off.

> You did not find the breakage in format-patch either to begin with; so
> your not finding does not give us much confidence that there is no other
> breakage, does it?
> 
> Grumble...

Sadly, this is an area that is not covered very well in the tests
(partially, I think, because it is "just" output which we tend to
neglect, and partially because the isatty() stuff is hard to test with
our harness). So I don't think it's _entirely_ Markus' fault.

-Peff

^ permalink raw reply

* [PATCH v2] Makefile: Use libc strlcpy on OSX >= 10.2
From: Benjamin Kramer @ 2009-01-24 19:01 UTC (permalink / raw)
  To: gitster; +Cc: git

OSX supports strlcpy(3) since 10.2 so we don't need
to use our own. This revised also patch checks the darwin
version. 10.0 is darwin v1.3; 10.1 is darwin v1.4.

Signed-off-by: Benjamin Kramer <benny.kra@gmail.com>
---
 Makefile |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index b4d9cb4..7dd5224 100644
--- a/Makefile
+++ b/Makefile
@@ -643,7 +643,9 @@ ifeq ($(uname_S),Darwin)
 	ifneq ($(shell expr "$(uname_R)" : '9\.'),2)
 		OLD_ICONV = UnfortunatelyYes
 	endif
-	NO_STRLCPY = YesPlease
+	ifeq ($(shell expr "$(uname_R)" : '1\.'),2)
+		NO_STRLCPY = YesPlease
+	endif
 	NO_MEMMEM = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
 endif
-- 
1.6.1.285.g3454

^ permalink raw reply related

* Re: [PATCH] cygwin: Convert paths for html help from posix to windows
From: Ramsay Jones @ 2009-01-24 18:51 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: Junio C Hamano, jaeckel, git
In-Reply-To: <20090122171605.GA16684@atjola.homenet>

Björn Steinbrink wrote:
> When using "git help --web" with cygwin, we used to pass the posix path
> to the browser, but a native windows browser will expect a windows path
> and is unable to make use of the given path.
> 
> So the cygwin port gets its own open_html implementation that handles
> the path conversion.
> 
> Reported-by: Steffen Jaeckel <jaeckel@stzedn.de>
> Tested-by: Steffen Jaeckel <jaeckel@stzedn.de>
> 
> Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
> ---
> OK, I don't really know if this is the right way to do it. Maybe when
> the browser was built for cygwin this breaks? I have no clue, it's
> admittedly just the result of a quick glance at the code and some
> googling to find the "right" cygwin function... :-/
> 

Hi Björn,

I had the same problem. However, rather than modifying git, I created a
firefox wrapper script (in ~/bin) which used the cygpath command line
tool to do the path conversion. Also, if you use "git instaweb", you
also need to filter out http URLs and pass them through un-molested
by cygpath (it turns http://localhost into http:\localhost).

My script is clearly a "quick hack" just to get something working for
me, but you may find it useful as a starting point for your own
("proper" ;-) script, so I've included it below.

HTH,
Ramsay Jones

-->8--
#!/bin/sh
#

while test $# != 0
do
    case "$1" in
	-version)
	    echo "firefox 1.5.0.2"
	    exit 0
	    ;;
	-new-tab)
	    echo "-new-tab not supported"
	    exit 1
	    ;;
	--)
	    shift
	    break
	    ;;
	-*)
	    echo "option '$1' not supported"
	    exit 1
	    ;;
	*)
	    break
	    ;;
    esac
    shift
done

if test "$1" = ""
then
	p=
else
	case "$1" in
	    http*)
		p="$1"
		;;
	    *)
		p="$(cygpath -w "$1")"
		;;
	esac
fi


"/cygdrive/c/Program Files/Mozilla Firefox/firefox.exe" "$p"

exit 0

^ permalink raw reply

* Re: [PATCH] Makefile: Use libc strlcpy on OSX
From: Benjamin Kramer @ 2009-01-24 18:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfxj8o8fq.fsf@gitster.siamese.dyndns.org>

On Sat, Jan 24, 2009 at 19:39, Junio C Hamano <gitster@pobox.com> wrote:
> How does your patch to Makefile refrain from kicking in for people with
> OSX older than 10.2, so that their builds are not broken?

I'm not even sure anyone uses <= 10.1 nowadays. But of course you're right,
I'll write a better patch.

-- Benjamin

^ 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