* [PATCH] githooks documentation: post-checkout hook is also called after clone
From: Jens Lehmann @ 2009-03-22 16:44 UTC (permalink / raw)
To: git; +Cc: gitster, peff
The documentation of the post-checkout hook just talks
about git-checkout. But recently git-clone was changed to
call it too, so we have to document that in githooks.txt.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
Documentation/githooks.txt | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 1fd512b..76deefc 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -151,6 +151,9 @@ indicating whether the checkout was a branch checkout (changing branches,
flag=1) or a file checkout (retrieving a file from the index, flag=0).
This hook cannot affect the outcome of 'git-checkout'.
+It is also run after 'git-clone'. The first parameter given to the hook is
+the null-ref, the second the ref of the new HEAD and the flag is always 1.
+
This hook can be used to perform repository validity checks, auto-display
differences from the previous HEAD if different, or set working dir metadata
properties.
--
1.6.2.1.275.ga797b
^ permalink raw reply related
* Re: What's cooking in git.git (Mar 2009, #06; Sat, 21)
From: Wincent Colaiuta @ 2009-03-22 15:57 UTC (permalink / raw)
To: David Aguilar; +Cc: Junio C Hamano, git
In-Reply-To: <20090321185817.GA22540@gmail.com>
El 21/3/2009, a las 19:58, David Aguilar escribió:
> On 0, Wincent Colaiuta <win@wincent.com> wrote:
>> El 21/3/2009, a las 8:58, Junio C Hamano escribió:
>>
>>> * da/difftool (Thu Mar 19 01:25:25 2009 -0700) 1 commit
>>> - difftool: move 'git-difftool' out of contrib
>>
>> Before this one goes any further, I noticed that nobody replied to my
>> email on this thread a few days ago:
>>
>> http://article.gmane.org/gmane.comp.version-control.git/113609
>>
>> My concern was:
>>
>>> Given that git-difftool shares basically all the same options as
>>> "git
>>> diff", I think a good long-term plan would be looking at adding the
>>> "--tool" option to "git diff" itself so that users wouldn't have to
>>> learn a new subcommand, just a new option.
>>
>>
>> What do people think?
>>
>> Cheers,
>> Wincent
>
> That could be interesting. git-difftool is just a
> frontend to git-diff so there isn't really any maintenance
> worries about keeping the options in sync. I do agree that
> keeping things easy for users is a noble goal and that that
> was your only concern.
>
> git-difftool is pure porcelain, so I'm interested in how we
> could implement this. Right now the call stack is:
>
> $ git difftool
> ... GIT_EXT_DIFF=git-difftool-helper
> ... git diff
> ... ... git-difftool-helper
> ... ... ... xxdiff
>
>
> What should it look like instead?
>
> Are you envisioning this (1):
>
> $ git diff --tool
> ... --tool was passed, so set GIT_EXT_DIFF?
> ... git-difftool-helper
> ... ... xxdiff ...
Yeah, something like that. I wasn't actually imagining that the logic
of git-difftool-helper would itself be rolled into "git diff". I was
just wondering if we could keep the command-count down.
Cheers,
Wincent
^ permalink raw reply
* Re: What's cooking in git.git (Mar 2009, #06; Sat, 21)
From: Wincent Colaiuta @ 2009-03-22 15:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vskl6ekve.fsf@gitster.siamese.dyndns.org>
El 21/3/2009, a las 20:28, Junio C Hamano escribió:
> Wincent Colaiuta <win@wincent.com> writes:
>
>>> Given that git-difftool shares basically all the same options as
>>> "git diff", I think a good long-term plan would be looking at adding
>>> the "--tool" option to "git diff" itself so that users wouldn't have
>>> to learn a new subcommand, just a new option.
>>
>> What do people think?
>
> I am not "people" but since I was "To:"ed not "Cc:"ed...
>
> I did not comment on it because personally I was not very interested
> in
> it; admittedly, I do not use difftool myself, but:
>
> (1) "git diff --tool" is more to type than "git difftool"; and
True, although an alias would fix that.
> (2) it requires adding more code to "git diff" for a dubious benefit
> from
> end user's point of view.
Fair enough. I was just wondering if we could avoid adding yet another
command to the already long list of installed commands. I understand
that "diff" is actually a thin dispatcher for different modes of
operation, but seeing as all of those modes basically boil down to
"show me the difference between two things", and that happens to be
the purpose of "difftool" as well, I thought it might make sense to
combine them.
> When an end user says "I want to compare two things with these
> settings
> (e.g. use color, with 5 lines of context, only inside Documentation/
> howto
> directory, detect renames with lower-than-usual threashold, ...)", the
> mental model is same whether the two things being compared happens
> to be
> index-vs-worktree or tree-vs-index from the end user's point of
> view. It
> makes a lot of sense for "git diff --options" to invoke both modes of
> operations with a similar-looking command line.
>
> Even though the --no-index mode of operation internally does not fit
> very
> well compared to the original four modes from the implementation
> point of
> view, it still naturally fits the end user's mental model and that
> is why
> it is given as an option to "git diff".
>
> Does "git difftool" fit well in the end user's mental model in a
> similar
> way to "git diff"? I somehow suspect it doesn't. What does it mean
> to
> give "-U8 --color --stat --summary -p --ignore-space-at-eol" options
> when
> you invoke it, for example?
Good question. Seeing as right now "difftool" just passes all options
through to "diff", I guess it's up to the user to pass in options
which actually make sense.
Cheers,
Wincent
^ permalink raw reply
* [RFC] git gui doesn't call post-checkout hook on checkout or clone
From: Jens Lehmann @ 2009-03-22 15:49 UTC (permalink / raw)
To: git; +Cc: gitster, spearce, peff
When checking out or cloning via git gui, the post-checkout
hook is not called. This is a bit unexpected ...
The reason is that git gui uses git read-tree with the -u
option and not git checkout and git clone. I changed git
read-tree to call the post-checkout hook when called with
-u and it seems to solve the problem. I would make a patch
for that if you want.
But is this the right way to do this? Seems like we could
surprise some users of git read-tree with this change in
behaviour.
(I hope i cc'ed the right people this time ;-)
Jens
^ permalink raw reply
* Re: [PATCH] Respect core.autocrlf when preparing temporary files for external diff
From: Sebastian Schuberth @ 2009-03-22 15:30 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <20090322074643.GA4826@coredump.intra.peff.net>
>> While the purist in me says #1 above is the right argument to make for
>> feeding "clean" version, I suspect that the textconv or extdiff tools more
>> often are not made from scratch and ported across platforms than are
>> cobbled up together out of tools the script writer finds on his platform.
>> I suspect that Dscho's "a tempfile should look like a checkout" would be
>> much friendlier to them in practice for this reason.
>
> I think you and I have about the same feeling on this, then. As somebody
> who does not actually use smudge/clean filters at all, I am willing to
> defer to Dscho's opinion, which is based on practical experience.
Me being the reporter of the original msysGit issue #177, I'd like to
clarify that my intention not necessarily was to make
"core.autocrlf=true" affect temporary files (i.e. to "smudge" them),
but to ensure that the files fed into "git diff" are always generated
/ acquired in a consistent way, so that they are in fact comparable.
I'd also be happy with a solution that always feeds clean files into
"git diff", although that would probably mean that we could not reuse
working tree files if "core.autocrlf=true" is set.
Maybe it's a good idea to look at how gitk displays the diff, for an
orientation. If the diff gitk shows is based on smudged files, git
diff should probably also always be fed with smudged files, and if the
diff gitk shows is based on clean files, git diff should probably also
always be fed with clean files.
--
Sebastian Schuberth
^ permalink raw reply
* Re: Disallow amending published commits?
From: Nicolas Sebrecht @ 2009-03-22 15:15 UTC (permalink / raw)
To: James Pickens; +Cc: Peter Harris, Git ML
In-Reply-To: <885649360903212109v316f441fvea3f498e91c0059e@mail.gmail.com>
On Sat, Mar 21, 2009 at 09:09:43PM -0700, James Pickens wrote:
> I think you understood the question perfectly, and your comments all make
> sense. Perhaps I'm just being paranoid and this won't be a problem at all.
I guess it's most depending of your proposed general workflow. So, it
makes sense.
> A bit of background might help explain my paranoia: I'm about to pilot Git
> on a fairly large project, where none of the users have Git experience, and
> many of them don't have much experience with any other version control
> system either.
As I understand, a part of your workflow is based on automatic testing
stages. It could be a good thing but I think you have to fit this into a
more general "human based worflow". I mean that parallel developments
should have one or more "official maintainers". Maintainers would have
to care of the history integrity, assume the responsability of passing
the tests, etc.
IMHO, good maintainers you can trust is much better than any "more
automatic restrictive testing suites".
Here is a link talking about that kind of issues that you (and your
maintainers) may be interested in:
http://kerneltrap.org/Linux/Git_Management
--
Nicolas Sebrecht
^ permalink raw reply
* Re: [PATCH] git-branch: display "was sha1" on branch deletion rather than just "sha1"
From: Brandon Casey @ 2009-03-22 15:11 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20090322061320.GB14765@coredump.intra.peff.net>
On Sun, Mar 22, 2009 at 1:13 AM, Jeff King <peff@peff.net> wrote:
> On Sat, Mar 21, 2009 at 07:09:17PM -0500, Brandon Casey wrote:
>
>> Make it more pleasant to read about a branch deletion by adding "was".
>> Jeff King suggested this, and I ignored it. He was right.
>
> Heh. While I am tempted to dance in a circle shouting "I told you so", I
> actually find that after getting used to it, I do not mind the current
> output. ;)
>
> Out of curiosity, what prompted your change of heart? Did you hear from
> somebody who found it confusing, or did you just change your mind?
I just changed my mind.
My original opinion was based on the output from 'git stash drop'. It
has the full sha1 in the message and looks like:
$git stash drop
Dropped refs/stash@{0} (58fa28fddd951deb782e3300b2d059f95544f6f1)
I like it. I like the full sha1.
For some reason, the partial sha1 in the branch delete message irritated me.
$ git branch -d mybranch
Deleted branch mybranch (455f59b).
I don't think it was because it was a partial sha1. Maybe it was
because the message seemed to imply that what was referenced by the
sha1 was being deleted. The branch is just a pointer, so deleting it
does not mean that what it pointed at was lost in any way. I think
adding "was" makes it seem more like a pointer to me. Or maybe it has
something to do with my mother, any psychology majors in the audience?
:)
-brandon
^ permalink raw reply
* Re: [PATCH v2 2/2] pack-objects: don't loosen objects available in alternate or kept packs
From: Brandon Casey @ 2009-03-22 14:48 UTC (permalink / raw)
To: Junio C Hamano, nico; +Cc: git
In-Reply-To: <7vbpru9nh9.fsf@gitster.siamese.dyndns.org>
On Sat, Mar 21, 2009 at 11:43 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Brandon Casey <drafnel@gmail.com> writes:
> Both patches are whitespace damaged, but I can cope.
I just retrieved one of the patches from gmane and some long lines
were wrapped. I tried out the gmail imap instructions from
SubmittingPatches for sending these patches. Those instructions say
that it is possible to send properly formatted patches through gmail,
and seem to instruct to use the web interface for actually sending the
patches. I wonder if there is some way to instruct gmail to not wrap
long lines? Or whether I did something else wrong?
Previously, I have used gmail's pop interface indirectly through my
phone provider which strips out the "From" field and replaces it with
one that only has my email address and not my name.
> But I am not sure about one thing...
>
>> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
>> index 6222f19..3f477c5 100644
>> --- a/builtin-pack-objects.c
>> +++ b/builtin-pack-objects.c
>> @@ -1944,6 +1944,29 @@ static void
>> add_objects_in_unpacked_packs(struct rev_info *revs)
>> free(in_pack.array);
>> }
>>
>> +static int has_sha1_pack_kept_or_nonlocal(const unsigned char *sha1)
>> +{
>> + static struct packed_git *last_found = (void *)1;
>> + struct packed_git *p;
>> +
>> + p = (last_found == (void *)1) ? packed_git : last_found;
>
> Why (void *)1, not like:
>
> static struct packed_git *last_found;
> struct packed_git *p = last_found ? last_found : packed_git;
>
> Am I missing something?
Heh, I am missing something too. Maybe I should have _thought_ more
about this code that I copied from sha1_file.c: find_pack_entry() and
I would have asked the same question about _that_ code.
Maybe Nico has some idea?
I'll send a new patch, unless Nico has some thoughts.
-brandon
^ permalink raw reply
* Re: [PATCH] remote: improve sorting of "configure for git push" list
From: Jay Soffian @ 2009-03-22 14:47 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Johannes Schindelin, Johannes Sixt,
Git Mailing List
In-Reply-To: <20090322085920.GA5201@coredump.intra.peff.net>
On Sun, Mar 22, 2009 at 4:59 AM, Jeff King <peff@peff.net> wrote:
> The data structure used to store this list is a string_list
> of sources with the destination in the util member. The
> current code just sorts on the source; if a single source is
> pushed to two different destination refs at a remote, then
> the order in which they are printed is non-deterministic.
>
> This patch implements a comparison using both fields.
> Besides being a little nicer on the eyes, giving a stable
> sort prevents false negatives in the test suite when
> comparing output.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> The discussion of the proper interface to the one-line wrapper to qsort
> didn't really resolve anything. It seems that the constraints of C make
> a nice wrapper a little painful, so let's just use a bare qsort. Once
> again, I really don't care what it looks like, so feel free to mark it
> up if you prefer differently; I just want it off my todo list, and to
> let JSixt run his tests in peace.
>
> I did have one somewhat sick thought for an API: have string_list's
> cmp_items (or an alternate cmp_items_with_util) treat a non-NULL util
> member as a pointer to a string, and use it as a secondary key in the
> sort. It works here because the first member of the push_info struct is
> the secondary key. But I think it is a bit too subtle for my taste.
Ack. There is one thing I wonder though. Is this even a sane config?
It's accepted, but is that intentional or just an accident? Would
anyone notice if the push parsing were changed to only accept a single
dest for a given source branch?
Thanks for the patch Jeff.
j.
^ permalink raw reply
* Re: [PATCH 1/7] check_ref_format(): tighten refname rules
From: Johannes Schindelin @ 2009-03-22 14:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1237673619-12608-2-git-send-email-gitster@pobox.com>
Hi,
On Sat, 21 Mar 2009, Junio C Hamano wrote:
> Yes, I know that tightening rules retroactively is bad, but this changes
> the rules for refnames to forbid:
Tightening rules retroactively is not only bad (if sometimes necessary),
but tightening rules without giving the user a chance to recover is really
bad.
'git branch -m' uses check_ref_format() to check the old name.
OTOH, a _warning_ should be plenty enough in most cases, and _not_ share
the shortcomings with tightening.
Ciao,
Dscho
^ permalink raw reply
* Re: Disallow amending published commits?
From: Peter Harris @ 2009-03-22 14:19 UTC (permalink / raw)
To: James Pickens; +Cc: Git ML
In-Reply-To: <885649360903212109v316f441fvea3f498e91c0059e@mail.gmail.com>
On Sun, Mar 22, 2009 at 12:09 AM, James Pickens wrote:
> I think you understood the question perfectly, and your comments all make
> sense. Perhaps I'm just being paranoid and this won't be a problem at all.
>
> A bit of background might help explain my paranoia: I'm about to pilot Git
> on a fairly large project, where none of the users have Git experience, and
> many of them don't have much experience with any other version control
> system either. I had to fight hard to get this pilot approved, and a lot
> of people will be watching to see how it goes, so I'm trying to do anything
> I can to make sure it will be successful.
Ah, yes. I can understand your paranoia.
Most new users will stick to your 'cheat sheet', and never even do
enough research to learn that you can amend existing history, much
less try it. A few will dig through the docs and try everything at
least once. I admit it; I fall into the latter category. :-)
Peter Harris
^ permalink raw reply
* Re: [PATCH] Makefile: turn on USE_ST_TIMESPEC for FreeBSD
From: Kjetil Barvik @ 2009-03-22 14:15 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20090322080847.GA9075@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Fixes broken compilation on FreeBSD 6.1.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> No idea if older versions support this, or if they just need NO_NSEC
> instead.
I google'd a litle and found this:
http://fuse4bsd.creo.hu/localcgi/man-cgi.cgi?lstat+2
http://www.nabble.com/st_mtimespec-or-st_mtime--td21217981.html
So, at least FreeBSD 6.2 also seems to need this. And I suspect that
OpenBSD and/or NetBSD maybe need a simmilar patch. Anybody who can
test this?
-- kjetil
> Makefile | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index aae3b09..320c897 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -713,6 +713,7 @@ ifeq ($(uname_S),FreeBSD)
> BASIC_CFLAGS += -I/usr/local/include
> BASIC_LDFLAGS += -L/usr/local/lib
> DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease
> + USE_ST_TIMESPEC = YesPlease
> THREADED_DELTA_SEARCH = YesPlease
> ifeq ($(shell expr "$(uname_R)" : '4\.'),2)
> PTHREAD_LIBS = -pthread
^ permalink raw reply
* [PATCH] documentation: Makefile accounts for SHELL_PATH setting
From: Ben Walton @ 2009-03-22 13:20 UTC (permalink / raw)
To: git; +Cc: Ben Walton
Ensure that the Makefile that generates and installs the Documentation
is aware of any SHELL_PATH setting. Use this value if found or the
current setting for SHELL if not. This is an accommodation for systems
where sh is not bash.
Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
---
Documentation/Makefile | 16 ++++++++++------
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 144ec32..0353690 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -67,6 +67,10 @@ ASCIIDOC_EXTRA += -a docbook-xsl-172
MANPAGE_XSL = manpage-1.72.xsl
endif
+SHELL_PATH ?= $(SHELL)
+# Shell quote;
+SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+
#
# Please note that there is a minor bug in asciidoc.
# The version after 6.0.3 _will_ include the patch found here:
@@ -116,7 +120,7 @@ install-pdf: pdf
$(INSTALL) -m 644 user-manual.pdf $(DESTDIR)$(pdfdir)
install-html: html
- sh ./install-webdoc.sh $(DESTDIR)$(htmldir)
+ '$(SHELL_PATH_SQ)' ./install-webdoc.sh $(DESTDIR)$(htmldir)
../GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
$(MAKE) -C ../ GIT-VERSION-FILE
@@ -178,7 +182,7 @@ user-manual.xml: user-manual.txt user-manual.conf
technical/api-index.txt: technical/api-index-skel.txt \
technical/api-index.sh $(patsubst %,%.txt,$(API_DOCS))
- cd technical && sh ./api-index.sh
+ cd technical && '$(SHELL_PATH_SQ)' ./api-index.sh
$(patsubst %,%.html,$(API_DOCS) technical/api-index): %.html : %.txt
$(ASCIIDOC) -b xhtml11 -f asciidoc.conf \
@@ -220,7 +224,7 @@ $(patsubst %.txt,%.texi,$(MAN_TXT)): %.texi : %.xml
howto-index.txt: howto-index.sh $(wildcard howto/*.txt)
$(RM) $@+ $@
- sh ./howto-index.sh $(wildcard howto/*.txt) >$@+
+ '$(SHELL_PATH_SQ)' ./howto-index.sh $(wildcard howto/*.txt) >$@+
mv $@+ $@
$(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt
@@ -234,14 +238,14 @@ $(patsubst %.txt,%.html,$(wildcard howto/*.txt)): %.html : %.txt
mv $@+ $@
install-webdoc : html
- sh ./install-webdoc.sh $(WEBDOC_DEST)
+ '$(SHELL_PATH_SQ)' ./install-webdoc.sh $(WEBDOC_DEST)
quick-install: quick-install-man
quick-install-man:
- sh ./install-doc-quick.sh $(DOC_REF) $(DESTDIR)$(mandir)
+ '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(DOC_REF) $(DESTDIR)$(mandir)
quick-install-html:
- sh ./install-doc-quick.sh $(HTML_REF) $(DESTDIR)$(htmldir)
+ '$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(HTML_REF) $(DESTDIR)$(htmldir)
.PHONY: .FORCE-GIT-VERSION-FILE
--
1.6.0.4
^ permalink raw reply related
* Re: [PATCH] builtin-fast-export.c: add default case to avoid crash on corrupt repo
From: Erik Faye-Lund @ 2009-03-22 12:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Erik Faye-Lund, git, Johannes Schindelin, Shawn O. Pearce
In-Reply-To: <40aa078e0903220522g66cf2172l9f1a43ed562cc4d3@mail.gmail.com>
> tag-object is NULL. Now, I'm no expert, but from browsing some code,
> it seems that "parse_object(tag->object.sha1)" should have been
> performed on the tag before looking up the tagged object. Does this
> make sense?
I guess I forgot to mention that adding a call to parse_object() does
make fast-exporting nested tags appear to work.
--
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656
^ permalink raw reply
* Re: [PATCH] builtin-fast-export.c: add default case to avoid crash on corrupt repo
From: Erik Faye-Lund @ 2009-03-22 12:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Erik Faye-Lund, git, Johannes Schindelin, Shawn O. Pearce
In-Reply-To: <7vd4cabffl.fsf@gitster.siamese.dyndns.org>
On Sun, Mar 22, 2009 at 12:54 AM, Junio C Hamano <gitster@pobox.com> wrote:
> A tag can point at anything, so this is not an issue about "crash on a
> _corrupt_ repository".
Ah, my bad. I wrongly assumed corrupted repos were the only ways of
triggering this issue. I quite easily managed to reproduce the crash
by setting up some tags to trees and tag objects to trees.
> I am not very familiar with this program, but the codepath involved should
> be prepared to _see_ any type of object instead of dying.
>
> What to do after _seeing_ a type of object is a different matter. It
> appears that there is no way to feed a tree object to fast-import, but I
> think the fast-import language can represent a tag that points at another
> tag just fine. So the best you can do is perhaps to issue a warning
> "skipping a tag that points at a tree object" and impoement a proper
> handling of a tag that points at a tag.
>
My patch simply applied the same error that was already present for
tags to tag objects, but yeah. Handling tagged tags and warning
instead of erroring-out makes more sense to me as well. I'll see if I
can write it up, and resubmit a patch.
After looking some more at the code, it seems that there's an attempt
to handle tags to tags there already, but it doesn't seem to work
properly; the program error out with a "fatal: Tag [tagname] points
nowhere?". This seems to be because the tagged-pointer of the second
tag-object is NULL. Now, I'm no expert, but from browsing some code,
it seems that "parse_object(tag->object.sha1)" should have been
performed on the tag before looking up the tagged object. Does this
make sense?
Also, I guess this calls for a couple of test-cases or something. I
haven't written any tests yet, so I might need some time to figuring
it out.
Anyway, I guess this makes the most sense as a four-patch series:
1) Add test-cases for tags of tag objects, tag objects of tag objects,
tags of trees and tag objects of trees.
2) Turn the existing error into a warning
3) Add the missing warning and remove the crash
4) Fix fast-export to export tags pointing to tags.
Makes sense?
Additionally, to reduce the chance of similar bugs in the future, the
code could be refactored a bit to have a handle_commit()-function, so
what goes on becomes a bit more obvious. Since this doesn't really
change any functionality, I guess it could be handed in as a separate
patch.
--
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656
^ permalink raw reply
* Re: [PATCH 1/8] builtin-apply: use warning() instead of fprintf(stderr, "warning: ")
From: Miklos Vajna @ 2009-03-22 11:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, John Tapsell, Git Mailing List
In-Reply-To: <7viqn5iqnn.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 680 bytes --]
On Thu, Feb 19, 2009 at 10:11:24PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
> > Other than that, these all look pretty straightforward. Probably the
> > shell scripts should be switched to match, too. But it would be nice to
> > hear from Junio first that this cleanup is even desired (so you don't
> > waste time).
>
> I think it is a good thing to do. If the pre-release-freeze is a good
> time to do so it is a different matter. A good way to judge would be
> how much of these overlap with "git diff master next" (smaller the better,
> obviously).
Should I rebase the series against current master and resend?
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [RFC/PATCH] git-shortlog: respect i18n.logOutputEncoding config setting
From: Miklos Vajna @ 2009-03-22 11:34 UTC (permalink / raw)
To: git
In-Reply-To: <1235092358-6895-1-git-send-email-vmiklos@frugalware.org>
[-- Attachment #1: Type: text/plain, Size: 463 bytes --]
On Fri, Feb 20, 2009 at 02:12:38AM +0100, Miklos Vajna <vmiklos@frugalware.org> wrote:
> As git-shortlog can be used as a filter as well, we do not really have
> the encoding info to do a reencode_string(), but in case
> i18n.logOutputEncoding is set, we can try to convert to the given value
> from utf-8.
Should I resend this patch without RFC in the subject?
I just hit this bug again today, then realised that I already posted a
patch for this problem. :-)
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH 3/7] check-ref-format --branch: give Porcelain a way to grok branch shorthand
From: Bert Wesarg @ 2009-03-22 10:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1237673619-12608-4-git-send-email-gitster@pobox.com>
On Sat, Mar 21, 2009 at 23:13, Junio C Hamano <gitster@pobox.com> wrote:
> The command may not be the best place to add this new feature, but
>
> $ git check-ref-format --branch "@{-1}"
>
> allows Porcelains to figure out what branch you were on before the last
> branch switching.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> builtin-check-ref-format.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/builtin-check-ref-format.c b/builtin-check-ref-format.c
> index 701de43..39db6cb 100644
> --- a/builtin-check-ref-format.c
> +++ b/builtin-check-ref-format.c
> @@ -5,9 +5,19 @@
> #include "cache.h"
> #include "refs.h"
> #include "builtin.h"
> +#include "strbuf.h"
>
> int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
> {
> + if (argc == 3 && !strcmp(argv[1], "--branch")) {
> + struct strbuf sb = STRBUF_INIT;
> + strbuf_branchname(&sb, argv[2]);
strbuf_branchname() will be introduced in the next patch!
Bert
> + strbuf_splice(&sb, 0, 0, "refs/heads/", 11);
> + if (check_ref_format(sb.buf))
> + die("'%s' is not a valid branch name", argv[2]);
> + printf("%s\n", sb.buf + 11);
> + exit(0);
> + }
> if (argc != 2)
> usage("git check-ref-format refname");
> return !!check_ref_format(argv[1]);
> --
> 1.6.2.1.299.gda643a
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH/RFD] builtin-revert.c: release index lock when cherry-picking an empty commit
From: Jeff King @ 2009-03-22 9:41 UTC (permalink / raw)
To: Mike Ralphson; +Cc: Junio C Hamano, git
In-Reply-To: <e2b179460903110408i4ab3c9cg3c863b89a2f57cba@mail.gmail.com>
[this is a follow-up on the "eval 'false\n\n'" returns 0 issue on
FreeBSD]
On Wed, Mar 11, 2009 at 11:08:06AM +0000, Mike Ralphson wrote:
> 2009/3/11 Jeff King <peff@peff.net>:
> > OK, then nothing to worry about there. I have no idea which shell
> > OpenBSD and NetBSD use these days, and I don't have access to a box.
> > Anybody?
>
> OpenBSD uses pdksh in Bourne shell mode for non-root shells (ksh mode
> for root) [1].
>
> NetBSD >=4 uses a Bourne shell but I don't know the exact provenance.
> [2] "A sh command appeared in Version 1 AT&T UNIX. It was, however,
> unmaintainable so we wrote this one."
>
> [1] http://www.openbsd.org/faq/faq10.html#ksh
> [2] http://www.netbsd.org/docs/misc/index.html#shells
Thanks for looking this up, Mike. It sounds like FreeBSD is probably the
only problematic one. I confirmed that the problem still exists in
FreeBSD 7.1, and I've mailed the git ports maintainer off-list to
make him aware of the issue. So we'll see what happens.
Junio, do you want to put anything in the release notes warning people
who build from source that this is a potential issue? Do you want
something in the Makefile detecting that the shell is broken?
-Peff
^ permalink raw reply
* [PATCH] everyday: use the dashless form of git-init
From: David Aguilar @ 2009-03-22 9:15 UTC (permalink / raw)
To: gitster; +Cc: git, David Aguilar
The 'Everyday GIT' guide was using the old dashed form
of git-init.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
Documentation/everyday.txt | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Documentation/everyday.txt b/Documentation/everyday.txt
index e598cdd..9310b65 100644
--- a/Documentation/everyday.txt
+++ b/Documentation/everyday.txt
@@ -98,7 +98,7 @@ Use a tarball as a starting point for a new repository.::
------------
$ tar zxf frotz.tar.gz
$ cd frotz
-$ git-init
+$ git init
$ git add . <1>
$ git commit -m "import of frotz source tree."
$ git tag v2.43 <2>
--
1.6.2.77.g8cc3f
^ permalink raw reply related
* [PATCH] remote: improve sorting of "configure for git push" list
From: Jeff King @ 2009-03-22 8:59 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Johannes Sixt, Jay Soffian, Git Mailing List
In-Reply-To: <20090319200308.GB17028@coredump.intra.peff.net>
The data structure used to store this list is a string_list
of sources with the destination in the util member. The
current code just sorts on the source; if a single source is
pushed to two different destination refs at a remote, then
the order in which they are printed is non-deterministic.
This patch implements a comparison using both fields.
Besides being a little nicer on the eyes, giving a stable
sort prevents false negatives in the test suite when
comparing output.
Signed-off-by: Jeff King <peff@peff.net>
---
The discussion of the proper interface to the one-line wrapper to qsort
didn't really resolve anything. It seems that the constraints of C make
a nice wrapper a little painful, so let's just use a bare qsort. Once
again, I really don't care what it looks like, so feel free to mark it
up if you prefer differently; I just want it off my todo list, and to
let JSixt run his tests in peace.
I did have one somewhat sick thought for an API: have string_list's
cmp_items (or an alternate cmp_items_with_util) treat a non-NULL util
member as a pointer to a string, and use it as a secondary key in the
sort. It works here because the first member of the push_info struct is
the secondary key. But I think it is a bit too subtle for my taste.
builtin-remote.c | 17 ++++++++++++++++-
1 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/builtin-remote.c b/builtin-remote.c
index 993acd6..9ef846f 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -922,6 +922,20 @@ int add_push_to_show_info(struct string_list_item *push_item, void *cb_data)
return 0;
}
+/*
+ * Sorting comparison for a string list that has push_info
+ * structs in its util field
+ */
+static int cmp_string_with_push(const void *va, const void *vb)
+{
+ const struct string_list_item *a = va;
+ const struct string_list_item *b = vb;
+ const struct push_info *a_push = a->util;
+ const struct push_info *b_push = b->util;
+ int cmp = strcmp(a->string, b->string);
+ return cmp ? cmp : strcmp(a_push->dest, b_push->dest);
+}
+
int show_push_info_item(struct string_list_item *item, void *cb_data)
{
struct show_info *show_info = cb_data;
@@ -1032,7 +1046,8 @@ static int show(int argc, const char **argv)
info.width = info.width2 = 0;
for_each_string_list(add_push_to_show_info, &states.push, &info);
- sort_string_list(info.list);
+ qsort(info.list->items, info.list->nr,
+ sizeof(*info.list->items), cmp_string_with_push);
if (info.list->nr)
printf(" Local ref%s configured for 'git push'%s:\n",
info.list->nr > 1 ? "s" : "",
--
1.6.2.1.276.gd47fa
^ permalink raw reply related
* [PATCH] Makefile: turn on USE_ST_TIMESPEC for FreeBSD
From: Jeff King @ 2009-03-22 8:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kjetil Barvik, git
Fixes broken compilation on FreeBSD 6.1.
Signed-off-by: Jeff King <peff@peff.net>
---
No idea if older versions support this, or if they just need NO_NSEC
instead.
Makefile | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index aae3b09..320c897 100644
--- a/Makefile
+++ b/Makefile
@@ -713,6 +713,7 @@ ifeq ($(uname_S),FreeBSD)
BASIC_CFLAGS += -I/usr/local/include
BASIC_LDFLAGS += -L/usr/local/lib
DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease
+ USE_ST_TIMESPEC = YesPlease
THREADED_DELTA_SEARCH = YesPlease
ifeq ($(shell expr "$(uname_R)" : '4\.'),2)
PTHREAD_LIBS = -pthread
--
1.6.2.1.276.gd47fa
^ permalink raw reply related
* Re: [PATCHv2 1/3] format-patch: create patch filename in one function
From: Junio C Hamano @ 2009-03-22 8:07 UTC (permalink / raw)
To: Stephen Boyd; +Cc: git
In-Reply-To: <49C5E113.3020601@gmail.com>
Stephen Boyd <bebarino@gmail.com> writes:
> Junio C Hamano wrote:
>> Stephen Boyd <bebarino@gmail.com> writes:
>>
>>> Junio C Hamano wrote:
>>> ...
>>>> IOW, you can introduce a new format specifier (say, "%f") to
>>>> format_commit_message() and the implemention of get_patch_filename() would
>>>> just prepare a strbuf and call format_commit_message() on it, no?
>>> This sounds great! I'm new so I don't know where to look for something
>>> like this.
>>
>> I suspect you may not even have to pass the generated string around if you
>> did so. Instead, you could pass the commit to log_write_email_headers()
>> instead of sha1_to_hex(commit->object.sha1) from show_log(), and use the
>> sha-1on the unix "From " line, and inside "if (opt->mime_boundar)", you
>> can ask format_commit_message("%f") to come up with a filename.
>
> I believe I won't be able to get the patch suffix at that point in the
> code. Unless I decide to add that to the rev_info instead?
Yeah, and I think that won't be "per commit" but "the same across the
traversal controlled by that rev_info", which is more in line with what
rev_info is about ;-)
^ permalink raw reply
* Re: [PATCH] Respect core.autocrlf when preparing temporary files for external diff
From: Jeff King @ 2009-03-22 7:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git, Sebastian Schuberth
In-Reply-To: <7v63i281py.fsf@gitster.siamese.dyndns.org>
On Sun, Mar 22, 2009 at 12:18:33AM -0700, Junio C Hamano wrote:
> Ok. Although I already queued the removal to 'pu' for tonight's pushout
> and it is way too late to revert that, I think I didn't have to remove the
> function. The codepath that lets you cheat by borrowing from the checkout
> runs convert_to_git() when it borrows, and if you are seeing a meaningful
> optimization even with that overhead, perhaps it would be worth keeping.
I certainly haven't done exhaustive tests. Obviously the one I did was a
bit contrived. I just think it makes sense to have numbers rather than
saying "this probably doesn't do anything anymore".
> While the purist in me says #1 above is the right argument to make for
> feeding "clean" version, I suspect that the textconv or extdiff tools more
> often are not made from scratch and ported across platforms than are
> cobbled up together out of tools the script writer finds on his platform.
> I suspect that Dscho's "a tempfile should look like a checkout" would be
> much friendlier to them in practice for this reason.
I think you and I have about the same feeling on this, then. As somebody
who does not actually use smudge/clean filters at all, I am willing to
defer to Dscho's opinion, which is based on practical experience.
> > For some reason, with your patch the tempfiles are created with mode
> > 0005 for me (whereas they are usually 0505), which makes open() in the
> > called script unhappy. Looking over the patch text, though, I have no
> > idea what change could be causing that.
>
> Neither 0005 nor 0505 sounds correct to me; shouldn't they be 0600 or
> something like that?
Argh, sorry, of course I should have realized something was wrong with
my baseline when I saw 0505. I was building on a half-finished WIP
related to textconv, which obviously is broken. Sorry for the noise.
-Peff
^ permalink raw reply
* Re: [PATCH] Respect core.autocrlf when preparing temporary files for external diff
From: Junio C Hamano @ 2009-03-22 7:18 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git, Sebastian Schuberth
In-Reply-To: <20090322061046.GA14765@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> ...how expensive is the check for convert_to_working_tree? It should
> just be a gitattributes lookup, shouldn't it? Which:
>
> a. we are doing anyway and caching
>
> b. which takes a fraction of a second (try "time git ls-files | git
> check-attr --stdin diff >/dev/null", which should give a
> worst-case).
Ok. Although I already queued the removal to 'pu' for tonight's pushout
and it is way too late to revert that, I think I didn't have to remove the
function. The codepath that lets you cheat by borrowing from the checkout
runs convert_to_git() when it borrows, and if you are seeing a meaningful
optimization even with that overhead, perhaps it would be worth keeping.
There is another check that should be there but is missing from the
current implementation of reuse_worktree_file(), other than the "is
convert_to_working_tree() a no-op for this path?" check. The last check
in the function would say "Yeah, we can reuse it" if the ce is marked
"assume unchanged"; we do not want to blindly reuse the file from the work
tree in that case.
> Anyway, I was planning to make a patch to always feed textconv the
> _clean_ version of each file. My thinking was:
>
> 1. Then tools get a consistent view of the data across platforms.
> I.e., my textconv munger or external diff script will work no
> matter what you think the working tree should look like.
>
> 2. The tool may want the clean version, or it may want the smudged
> version. Or it may be able to operate on either. If we give it a
> format it doesn't like, it will have to undo whatever we did.
>
> For most cases, we start with the clean file (i.e., from a tree or
> from the index). If we hand out the clean file and the script
> doesn't like it, it pays the cost to smudge once. If we hand it the
> smudged file and the script doesn't like it, we pay the cost to
> smudge _and_ the script pays the cost to clean.
While the purist in me says #1 above is the right argument to make for
feeding "clean" version, I suspect that the textconv or extdiff tools more
often are not made from scratch and ported across platforms than are
cobbled up together out of tools the script writer finds on his platform.
I suspect that Dscho's "a tempfile should look like a checkout" would be
much friendlier to them in practice for this reason.
> For some reason, with your patch the tempfiles are created with mode
> 0005 for me (whereas they are usually 0505), which makes open() in the
> called script unhappy. Looking over the patch text, though, I have no
> idea what change could be causing that.
Neither 0005 nor 0505 sounds correct to me; shouldn't they be 0600 or
something like that?
^ 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