* Re: Git Notes idea.
From: Jeff King @ 2008-12-18 13:54 UTC (permalink / raw)
To: Govind Salinas; +Cc: Git Mailing List, Johannes Schindelin
In-Reply-To: <5d46db230812170906h7fdcac03o60386504c8df1083@mail.gmail.com>
On Wed, Dec 17, 2008 at 11:06:15AM -0600, Govind Salinas wrote:
> Yes, I was thinking that this is the natural way to do things, save that I
> would be lazy loading the trees into a cache instead of caching them
> all up front. This is one of the reasons that I think the fan out will
> help.
I was working under the assumption that you are going to do multiple
note lookups. If you are, then the fan-out isn't really going to help,
as you're going to end up pulling in all of the subtrees anyway. It
helps some if you're only doing a single lookup, but I don't know if
that is measurable.
> Yes, I completely agree that I want it to have the same scheme as what
> git will use. That is the reason I posted this here. Since no method
> has been formally accepted (checked into master) I wanted to see if
> I could nudge things along. I wasn't aware that you and Dscho had
> a (very similar) plan. Please, if you guys are decided on the format
> then I can just go off and start working on it. But it sounds like there
> isn't consensus yet.
This is probably not the answer you want, but I think the final design
depends on some C experiments. For example, whether or not there should
be fan-out depends on how it affects performance, which means we need to
do at least partial implementations to compare. So it really is just
waiting for somebody to sit down and do it.
> I like the overall plan, but I would suggest that --notes[=default] and
> --note-type=whatever would be a little friendlier and less error prone.
But keeping it as a single string means there is no ambiguity when you
specify multiple notes at once. For example:
git log \
--note-filter='test:status == "fail" && importance > 3' \
--pretty=format:%h%n%N(test:errors)
would do something like:
foreach commit $C
compare refs/notes/test:$C/status against the string "fail"
compare refs/notes/default:$C/importance against the number 3
if either don't match, skip the commit
show the hash and the contents of refs/notes/test:$C/errors
and obviously that filter language is totally made up and we may or may
not want to do something that complex. But my point is that we are
defining a namespace of notes, and we want to be able to refer to a
multiple fully qualified names.
-Peff
^ permalink raw reply
* Re: is it possible filter the revision history of a single file into another repository?
From: Thomas Jarosch @ 2008-12-18 14:04 UTC (permalink / raw)
To: Whit Armstrong; +Cc: git
In-Reply-To: <8ec76080812180551p8c97a0dqa2025e67792946c7@mail.gmail.com>
On Thursday, 18. December 2008 14:51:12 Whit Armstrong wrote:
> For instance, if my repository contains foo.c, and 100 other files.
>
> I would like to create a new and separate repository containing only
> the revision history of foo.c.
>
> Would someone mind pointing me at some documentation for this
> procedure if it exists?
This worked for me:
git filter-branch --tag-name-filter cat --index-filter \
'git ls-files -s |grep -P "\t(DIR1|DIR2)" \
|GIT_INDEX_FILE=$GIT_INDEX_FILE.new git update-index --index-info &&
mv $GIT_INDEX_FILE.new $GIT_INDEX_FILE' -- --all
Run "git ls-files -s" to see the output format.
Replace the "DIR1|DIR2" with "foo.c".
Later on you might want to remove empty commits from the history:
git filter-branch --tag-name-filter cat --commit-filter 'if [ z$1 = z`git rev-parse $3^{tree}` ]; then skip_commit "$@"; else git commit-tree "$@"; fi' "$@" -- --all
If you want to run two filter-branch commands in a row
or you want to free up the space in .git afterwards:
- git for-each-ref --format='%(refname)' refs/original | xargs -i git update-ref -d {}
- git reflog expire --expire=0 --all
- git repack -a -d
- git prune
Cheers,
Thomas
^ permalink raw reply
* Re: [PATCH] Add git-edit-index.perl
From: Jeff King @ 2008-12-18 14:04 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Neil Roberts, git
In-Reply-To: <alpine.DEB.1.00.0812181446430.6952@intel-tinevez-2-302>
On Thu, Dec 18, 2008 at 02:48:39PM +0100, Johannes Schindelin wrote:
> Yes, it is a neat idea. But I always keep in mind what Junio had to say
> about my "add -e" thing (that I use pretty frequently myself): you will
> put something into the index that has _never_ been tested.
>
> Would we really want to bless such a workflow with "official" support?
That is definitely something to be concerned about. Which is why my
workflow is something like:
$ hack hack hack
$ while ! git diff; do
git add -p
git commit
done
$ for i in `git rev-list origin..`; do
git checkout $i && make test || barf
done
That is, it is not inherently a problem to put something untested into
the index as long as you are doing it so that you can go back and test
later.
It _would_ be a nicer workflow to say "I don't want these changes yet"
and selectively put them elsewhere, test what's in the working tree,
commit, and then grab some more changes from your stash. But we don't
have interactive stashing and unstashing yet, which would be required
for that.
-Peff
^ permalink raw reply
* Re: is it possible filter the revision history of a single file into another repository?
From: Sverre Rabbelier @ 2008-12-18 14:15 UTC (permalink / raw)
To: Whit Armstrong; +Cc: git
In-Reply-To: <8ec76080812180551p8c97a0dqa2025e67792946c7@mail.gmail.com>
On Thu, Dec 18, 2008 at 14:51, Whit Armstrong <armstrong.whit@gmail.com> wrote:
> For instance, if my repository contains foo.c, and 100 other files.
>
> I would like to create a new and separate repository containing only
> the revision history of foo.c.
>
> Would someone mind pointing me at some documentation for this
> procedure if it exists?
I think "git filter-branch" is what you need. Have it filter out
changes to files but foo.c, and then remove all empty commits.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: is it possible filter the revision history of a single file into another repository?
From: Whit Armstrong @ 2008-12-18 14:19 UTC (permalink / raw)
To: Thomas Jarosch; +Cc: git
In-Reply-To: <200812181504.02105.thomas.jarosch@intra2net.com>
thanks, I will give this a try.
On Thu, Dec 18, 2008 at 9:04 AM, Thomas Jarosch
<thomas.jarosch@intra2net.com> wrote:
> On Thursday, 18. December 2008 14:51:12 Whit Armstrong wrote:
>> For instance, if my repository contains foo.c, and 100 other files.
>>
>> I would like to create a new and separate repository containing only
>> the revision history of foo.c.
>>
>> Would someone mind pointing me at some documentation for this
>> procedure if it exists?
>
> This worked for me:
>
> git filter-branch --tag-name-filter cat --index-filter \
> 'git ls-files -s |grep -P "\t(DIR1|DIR2)" \
> |GIT_INDEX_FILE=$GIT_INDEX_FILE.new git update-index --index-info &&
> mv $GIT_INDEX_FILE.new $GIT_INDEX_FILE' -- --all
>
> Run "git ls-files -s" to see the output format.
> Replace the "DIR1|DIR2" with "foo.c".
>
> Later on you might want to remove empty commits from the history:
> git filter-branch --tag-name-filter cat --commit-filter 'if [ z$1 = z`git rev-parse $3^{tree}` ]; then skip_commit "$@"; else git commit-tree "$@"; fi' "$@" -- --all
>
> If you want to run two filter-branch commands in a row
> or you want to free up the space in .git afterwards:
>
> - git for-each-ref --format='%(refname)' refs/original | xargs -i git update-ref -d {}
> - git reflog expire --expire=0 --all
> - git repack -a -d
> - git prune
>
> Cheers,
> Thomas
>
>
^ permalink raw reply
* Re: [PATCH][RESEND] guilt: add option guilt.diffstat
From: Josef Jeff Sipek @ 2008-12-18 14:39 UTC (permalink / raw)
To: Wu Fengguang; +Cc: git@vger.kernel.org, Boyd Stephen Smith Jr.
In-Reply-To: <20081218112643.GA15416@localhost>
On Thu, Dec 18, 2008 at 07:26:43PM +0800, Wu Fengguang wrote:
> Introduce option guilt.diffstat so that we don't have to type
> "guilt refresh --diffstat" in its full form every time.
I haven't forgotten about this. I'll try to apply it tonight/tomorrow
morning.
Jeff.
--
Humans were created by water to transport it upward.
^ permalink raw reply
* Re: [PATCHv5 2/3] gitweb: add patches view
From: Giuseppe Bilotta @ 2008-12-18 16:23 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, git, Petr Baudis
In-Reply-To: <200812181033.57360.jnareb@gmail.com>
On Thu, Dec 18, 2008 at 10:33 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Thu, 18 Dec 2008, Junio C Hamano wrote:
>> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
>>
>> > sub git_commitdiff {
>> > my $format = shift || 'html';
>> > + my %params = @_;
>> > ...
>> > + if ($params{-single}) {
>> > + push @commit_spec, '-1';
>> > + } else {
>> > + if ($patch_max > 0) {
>> > ...
>> > + }
>> > @@ -5625,6 +5635,10 @@ sub git_commitdiff_plain {
>> >
>> > # format-patch-style patches
>> > sub git_patch {
>> > + git_commitdiff('patch', -single=> 1);
>> > +}
>>
>> Hmm, if you are changing the interface this way, wouldn't it make more
>> sense to also do this?
>>
>> git_commitdiff(--format => 'patch', --single => 1);
>> git_commitdiff(--format => 'html');
>
> The first argument (format) is _required_, second is _optional_;
> I'd rather use named parameters trick only for optional parameters.
> Because with more than one optional parameter function call begins
> to be cryptic; also flag (boolean) parameters are more readable
> when used as named parameters.
I have mixed feelings about this: on the one hand we have href() (say)
that takes all its params from a has, but on the other hand we have
esc_html() (say) that takes only additional options from a hash. I'm
personally more inclined towards the latter usage for git_commitdiff
(i.e. this patch) but since the other alternative is straightforward I
sent a v6 of the patchset which implements it.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: [PATCH] Add git-edit-index.perl
From: Johannes Schindelin @ 2008-12-18 16:24 UTC (permalink / raw)
To: Jeff King; +Cc: Neil Roberts, git
In-Reply-To: <20081218140411.GB6706@coredump.intra.peff.net>
Hi,
On Thu, 18 Dec 2008, Jeff King wrote:
> It _would_ be a nicer workflow to say "I don't want these changes yet"
> and selectively put them elsewhere, test what's in the working tree,
> commit, and then grab some more changes from your stash. But we don't
> have interactive stashing and unstashing yet, which would be required
> for that.
git stash -i... Yes, I'd like that!
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Add git-edit-index.perl
From: Miklos Vajna @ 2008-12-18 16:36 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jeff King, Neil Roberts, git
In-Reply-To: <alpine.DEB.1.00.0812181723340.6952@intel-tinevez-2-302>
[-- Attachment #1: Type: text/plain, Size: 698 bytes --]
On Thu, Dec 18, 2008 at 05:24:00PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Thu, 18 Dec 2008, Jeff King wrote:
>
> > It _would_ be a nicer workflow to say "I don't want these changes yet"
> > and selectively put them elsewhere, test what's in the working tree,
> > commit, and then grab some more changes from your stash. But we don't
> > have interactive stashing and unstashing yet, which would be required
> > for that.
>
> git stash -i... Yes, I'd like that!
Or git checkout -i?
For the cases when you did two changes in a file, you realize one of
them is not yet necessary, but you don't want to stage/commit the other
change yet.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* git-fast-export and tags without a tagger
From: Miklos Vajna @ 2008-12-18 16:46 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, scott
[-- Attachment #1: Type: text/plain, Size: 803 bytes --]
Hi,
Tags created with ancient versions of git have no tagger. The udev repo
has such tags, for example:
$ git cat-file tag 4ea98ca6db3b84f5bc16eac8574e5c209ec823ce
object face198a5f21027fefe796dc01e19e349a2d36ce
type commit
tag 062
fast-export will fail on these repos. From IRC:
20:05 < Keybuk> vmiklos: exactly the same error with 1.6.0.5
20:07 < Keybuk> $ git fast-export --signed-tag=strip --all
20:07 < Keybuk> fatal: No tagger for tag 4ea98ca6db3b84f5bc16eac8574e5c209ec823ce
I think it would be nice to handle the situation better than just die().
What about a --force option that would fake the tagger in case the tag
points to a commmit and use the commiter from there?
I'm willing to work on this, but I thought it's better to discuss the
right solution for the problem first.
Thanks.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH 4/5] Make 'diff_populate_filespec()' use the new 'strbuf_readlink()'
From: Linus Torvalds @ 2008-12-18 16:55 UTC (permalink / raw)
To: Mark Burton; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20081218121118.3635c53c@crow>
On Thu, 18 Dec 2008, Mark Burton wrote:
>
> Does it need a cast on some architectures?
Gaah. My bad. It should work fine ("unsigned long" is physically the same
type as "size_t" in your case), but on 32-bit x86, size_t is generally
"unsigned int" - which is the same physical type there (both int and long
are 32-bit) but causes a valid warning.
I think we should just make the "size" member "size_t". I use "unsigned
long" out of much too long habit, since we traditionally avoided "size_t"
in the kernel due to it just being another unnecessary architecture-
specific detail.
So the proper patch is probably just the following. Sorry about that,
Linus
---
diffcore.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/diffcore.h b/diffcore.h
index 5b63458..16a73e6 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -30,7 +30,7 @@ struct diff_filespec {
void *data;
void *cnt_data;
const char *funcname_pattern_ident;
- unsigned long size;
+ size_t size;
int count; /* Reference count */
int xfrm_flags; /* for use by the xfrm */
int rename_used; /* Count of rename users */
^ permalink raw reply related
* Re: [PATCH 4/5] Make 'diff_populate_filespec()' use the new 'strbuf_readlink()'
From: René Scharfe @ 2008-12-18 16:56 UTC (permalink / raw)
To: Mark Burton; +Cc: Linus Torvalds, Junio C Hamano, Git Mailing List
In-Reply-To: <20081218121118.3635c53c@crow>
Mark Burton schrieb:
> Howdy folks,
>
> When I compile this latest version of diff.c on a i686 dual-core Pentium box
> I see:
>
> diff.c: In function ‘diff_populate_filespec’:
> diff.c:1781: warning: passing argument 2 of ‘strbuf_detach’ from incompatible pointer type
>
> The same code compiles without warning on a x86_64 AMD box. Both
> machines are running stock Ubuntu 8.04.
>
> Does it need a cast on some architectures?
The type of the size member of struct stat is off_t, while strbuf_detach expects
a size_t pointer. This patch should fix the warning:
diff --git a/diff.c b/diff.c
index f160c1a..0484601 100644
--- a/diff.c
+++ b/diff.c
@@ -1778,7 +1778,8 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)
if (strbuf_readlink(&sb, s->path, s->size))
goto err_empty;
- s->data = strbuf_detach(&sb, &s->size);
+ s->size = sb.len;
+ s->data = strbuf_detach(&sb, NULL);
s->should_free = 1;
return 0;
}
^ permalink raw reply related
* [PATCH] test overlapping ignore patterns
From: Michael J Gruber @ 2008-12-18 17:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <7viqpjra57.fsf@gitster.siamese.dyndns.org>
Add a test which checks that negated patterns such as "!foo.html" can
override previous patterns such as "*.html". This is documented
behaviour but had not been tested so far.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
t/t3001-ls-files-others-exclude.sh | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
index 8666946..85aef12 100755
--- a/t/t3001-ls-files-others-exclude.sh
+++ b/t/t3001-ls-files-others-exclude.sh
@@ -140,4 +140,10 @@ test_expect_success 'trailing slash in exclude forces directory match (2)' '
'
+test_expect_success 'negated exclude matches can override previous ones' '
+
+ git ls-files --others --exclude="a.*" --exclude="!a.1" >output &&
+ grep "^a.1" output
+'
+
test_done
--
1.6.1.rc1
^ permalink raw reply related
* Re: [PATCH 4/5] Make 'diff_populate_filespec()' use the new 'strbuf_readlink()'
From: René Scharfe @ 2008-12-18 17:28 UTC (permalink / raw)
Cc: Mark Burton, Linus Torvalds, Junio C Hamano, Git Mailing List
In-Reply-To: <494A80D3.7070605@lsrfire.ath.cx>
René Scharfe schrieb:
> Mark Burton schrieb:
>> Howdy folks,
>>
>> When I compile this latest version of diff.c on a i686 dual-core Pentium box
>> I see:
>>
>> diff.c: In function ‘diff_populate_filespec’:
>> diff.c:1781: warning: passing argument 2 of ‘strbuf_detach’ from incompatible pointer type
>>
>> The same code compiles without warning on a x86_64 AMD box. Both
>> machines are running stock Ubuntu 8.04.
>>
>> Does it need a cast on some architectures?
>
> The type of the size member of struct stat is off_t, while strbuf_detach expects
> a size_t pointer. This patch should fix the warning:
Nevermind, I somehow missed the last "t" in line 1760.. The patch works
nevertheless. 8-)
René
^ permalink raw reply
* Re: [PATCHv5 2/3] gitweb: add patches view
From: Jakub Narebski @ 2008-12-18 17:28 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: Junio C Hamano, git, Petr Baudis
In-Reply-To: <cb7bb73a0812180823y5249abcakd419d4fee9942d84@mail.gmail.com>
On Thu, Dec 18, 2008, Giuseppe Bilotta wrote:
> On Thu, Dec 18, 2008, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Thu, 18 Dec 2008, Junio C Hamano wrote:
>>> [...] wouldn't it make more sense to also do this?
>>>
>>> git_commitdiff(--format => 'patch', --single => 1);
>>> git_commitdiff(--format => 'html');
>>
>> The first argument (format) is _required_, second is _optional_;
>> I'd rather use named parameters trick only for optional parameters.
>> Because with more than one optional parameter function call begins
>> to be cryptic; also flag (boolean) parameters are more readable
>> when used as named parameters.
>
> I have mixed feelings about this: on the one hand we have href() (say)
> that takes all its params from a has, but on the other hand we have
> esc_html() (say) that takes only additional options from a hash. [...]
It is not "on one hand". href() is a bit special case. It has _all_
parameters optional, especially now that -replay=>1 was introduced.
It has no required parameters. Therefore all parameters are named.
Beside, href() mimics a bit CGI interface...
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 4/5] Make 'diff_populate_filespec()' use the new 'strbuf_readlink()'
From: René Scharfe @ 2008-12-18 17:41 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Mark Burton, Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0812180851120.14014@localhost.localdomain>
Linus Torvalds schrieb:
>
> On Thu, 18 Dec 2008, Mark Burton wrote:
>> Does it need a cast on some architectures?
>
> Gaah. My bad. It should work fine ("unsigned long" is physically the same
> type as "size_t" in your case), but on 32-bit x86, size_t is generally
> "unsigned int" - which is the same physical type there (both int and long
> are 32-bit) but causes a valid warning.
>
> I think we should just make the "size" member "size_t". I use "unsigned
> long" out of much too long habit, since we traditionally avoided "size_t"
> in the kernel due to it just being another unnecessary architecture-
> specific detail.
>
> So the proper patch is probably just the following. Sorry about that,
>
> Linus
> ---
> diffcore.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/diffcore.h b/diffcore.h
> index 5b63458..16a73e6 100644
> --- a/diffcore.h
> +++ b/diffcore.h
> @@ -30,7 +30,7 @@ struct diff_filespec {
> void *data;
> void *cnt_data;
> const char *funcname_pattern_ident;
> - unsigned long size;
> + size_t size;
> int count; /* Reference count */
> int xfrm_flags; /* for use by the xfrm */
> int rename_used; /* Count of rename users */
Yes, but now I get two new warnings:
diff.c: In function `diff_populate_filespec':
diff.c:1809: warning: passing arg 2 of `sha1_object_info' from
incompatible pointer type
diff.c:1811: warning: passing arg 3 of `read_sha1_file' from
incompatible pointer type
If we followed that way along we'd convert just about everything to use
size_t, which is going a bit too far during the -rc phase..
René
PS: In the other subthread, I was missing the "t" in "st" in line 1757,
not 1760. Ahem.
^ permalink raw reply
* Re: [PATCH 4/5] Make 'diff_populate_filespec()' use the new 'strbuf_readlink()'
From: Linus Torvalds @ 2008-12-18 17:49 UTC (permalink / raw)
To: René Scharfe; +Cc: Mark Burton, Junio C Hamano, Git Mailing List
In-Reply-To: <494A8B57.6070106@lsrfire.ath.cx>
On Thu, 18 Dec 2008, René Scharfe wrote:
>
> Yes, but now I get two new warnings:
>
> diff.c: In function `diff_populate_filespec':
> diff.c:1809: warning: passing arg 2 of `sha1_object_info' from
> incompatible pointer type
> diff.c:1811: warning: passing arg 3 of `read_sha1_file' from
> incompatible pointer type
Yeah, yeah, we should probably fix it all, but you're right, your patch
avoids the pain.
The good news is that "size_t" and "unsigned long" really are the same
size on all sane platforms. You'll see differences just in the odd 16-bit
world ("small" memory model with 16-bit size_t and possibly 32-bit "long")
Although I guess some really odd more modern case might have a 32-bit
pointer (and size_t) and 64-bit long. If it's possible to screw the type
system up, history shows that somebody (usually Windows) will do it.
Linus
^ permalink raw reply
* Re: [PATCH 4/5] Make 'diff_populate_filespec()' use the new 'strbuf_readlink()'
From: Olivier Galibert @ 2008-12-18 17:56 UTC (permalink / raw)
To: Linus Torvalds
Cc: René Scharfe, Mark Burton, Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0812180945330.14014@localhost.localdomain>
On Thu, Dec 18, 2008 at 09:49:23AM -0800, Linus Torvalds wrote:
> If it's possible to screw the type system up, history shows that
> somebody (usually Windows) will do it.
AFAIK, the Win64 long is 32 bits and size_t/void * is 64 bits. I'll
leave you to your groaning.
OG.
^ permalink raw reply
* Re: [PATCHv5 2/3] gitweb: add patches view
From: Jakub Narebski @ 2008-12-18 18:15 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: Junio C Hamano, git, Petr Baudis
In-Reply-To: <7v4p12hv5q.fsf@gitster.siamese.dyndns.org>
On Thu, 18 Dec 2008, Junio C Hamano wrote:
> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
>
> > sub git_commitdiff {
> > my $format = shift || 'html';
> > + my %params = @_;
> > ...
> > + if ($params{-single}) {
> > + push @commit_spec, '-1';
> > + } else {
> > + if ($patch_max > 0) {
> > ...
> > + }
> > @@ -5625,6 +5635,10 @@ sub git_commitdiff_plain {
> >
> > # format-patch-style patches
> > sub git_patch {
> > + git_commitdiff('patch', -single=> 1);
> > +}
>
> Hmm, if you are changing the interface this way, wouldn't it make more
> sense to also do this?
>
> git_commitdiff(--format => 'patch', --single => 1);
> git_commitdiff(--format => 'html');
Just a though, but does it really take much sense to have "patch" and
"patches" views handled in git_commitdiff? I can understand in the
first version, where it was more about better 'commitdiff_plain', but
I wonder about now, where patch(es) view is something between git show
and log_plain format... Wouldn't it be simpler to use separate
subroutine, for example git_format_patch or something?
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCHv5 2/3] gitweb: add patches view
From: Junio C Hamano @ 2008-12-18 19:15 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Giuseppe Bilotta, git, Petr Baudis
In-Reply-To: <200812181033.57360.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> On Thu, 18 Dec 2008, Junio C Hamano wrote:
>> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
>>
>> > sub git_commitdiff {
>> > my $format = shift || 'html';
>> > + my %params = @_;
>> > ...
>> > + if ($params{-single}) {
>> > + push @commit_spec, '-1';
>> > + } else {
>> > + if ($patch_max > 0) {
>> > ...
>> > + }
>> > @@ -5625,6 +5635,10 @@ sub git_commitdiff_plain {
>> >
>> > # format-patch-style patches
>> > sub git_patch {
>> > + git_commitdiff('patch', -single=> 1);
>> > +}
>>
>> Hmm, if you are changing the interface this way, wouldn't it make more
>> sense to also do this?
>>
>> git_commitdiff(--format => 'patch', --single => 1);
>> git_commitdiff(--format => 'html');
>
> The first argument (format) is _required_, second is _optional_;
My reading of the first line of the sub seems to contradict with your
statement. "If the first argument is empty, we use 'html' instead" sounds
pretty optional to me.
^ permalink raw reply
* Re: git-fast-export and tags without a tagger
From: Junio C Hamano @ 2008-12-18 19:15 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Johannes Schindelin, git, scott
In-Reply-To: <20081218164614.GS5691@genesis.frugalware.org>
Miklos Vajna <vmiklos@frugalware.org> writes:
> Tags created with ancient versions of git have no tagger. The udev repo
> has such tags, for example:
>
> $ git cat-file tag 4ea98ca6db3b84f5bc16eac8574e5c209ec823ce
> object face198a5f21027fefe796dc01e19e349a2d36ce
> type commit
> tag 062
>
> fast-export will fail on these repos. From IRC:
Is "fast-export" the only thing that chokes on these tags?
What I am worried about is if we have accidentally/stupidly/by mistake
made other codepaths that check the validity of a tag object too strict,
which would result things like "git show $such_a_tag" to barf.
^ permalink raw reply
* Git with Hudson
From: Indy Nagpal @ 2008-12-18 19:23 UTC (permalink / raw)
To: git
Hi
Currently we use Subversion as our VCS. This ties into our CI process
that uses Hudson.
I've been using git svn with great success over the last few months
and a few our developers have also started using it. We now almost
ready to switch our repository from SVN to Git.
However, before we do that I wanted to check if anyone has had any
experience/feedback in integrating Git with Hudson CI server?
Thanks
Indy
PS: Apologies if this is posted a couple of times to the list. Gmail
is refusing to send the email to the mailing list address, so trying
to send it in a couple of different ways
--
Indiver Nagpal
Straker Interactive
^ permalink raw reply
* Re: [PATCHv5 2/3] gitweb: add patches view
From: Jakub Narebski @ 2008-12-18 19:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Giuseppe Bilotta, git, Petr Baudis
In-Reply-To: <7viqphguqu.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>> On Thu, 18 Dec 2008, Junio C Hamano wrote:
>>> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
>>>
>>>> sub git_commitdiff {
>>>> my $format = shift || 'html';
>>>> + my %params = @_;
>>>> ...
>>>> + if ($params{-single}) {
>>>> + push @commit_spec, '-1';
>>>> + } else {
>>>> + if ($patch_max> 0) {
>>>> ...
>>>> + }
>>>> @@ -5625,6 +5635,10 @@ sub git_commitdiff_plain {
>>>>
>>>> # format-patch-style patches
>>>> sub git_patch {
>>>> + git_commitdiff('patch', -single=> 1);
>>>> +}
>>>
>>> Hmm, if you are changing the interface this way, wouldn't it make more
>>> sense to also do this?
>>>
>>> git_commitdiff(--format => 'patch', --single => 1);
>>> git_commitdiff(--format => 'html');
>>
>> The first argument (format) is _required_, second is _optional_;
>
> My reading of the first line of the sub seems to contradict with your
> statement. "If the first argument is empty, we use 'html' instead" sounds
> pretty optional to me.
Well, for me it is more of required argument, but with default value.
But now we argue semantics... :-)
--
Jakub Narebski
Poland
^ permalink raw reply
* [PATCH] fast-export: deal with tag objects that do not have a tagger
From: Johannes Schindelin @ 2008-12-18 19:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Miklos Vajna, git, scott
In-Reply-To: <7vbpv9guqd.fsf@gitster.siamese.dyndns.org>
When no tagger was found (old Git produced tags like this), a
tagger "No Tagger <no-tagger>" with a tag date of the beginning of
(Unix) time is output, so that fast-import is
still able to import the result.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
On Thu, 18 Dec 2008, Junio C Hamano wrote:
> Miklos Vajna <vmiklos@frugalware.org> writes:
>
> > Tags created with ancient versions of git have no tagger. The
> > udev repo has such tags, for example:
> >
> > $ git cat-file tag 4ea98ca6db3b84f5bc16eac8574e5c209ec823ce
> > object face198a5f21027fefe796dc01e19e349a2d36ce
> > type commit
> > tag 062
> >
> > fast-export will fail on these repos. From IRC:
>
> Is "fast-export" the only thing that chokes on these tags?
I think so. The responsible code is in fast-export.c, in any
case. Of course, fast-import refuses to import a tag without a
tagger, so...
builtin-fast-export.c | 12 ++++++++----
t/t9301-fast-export.sh | 15 +++++++++++++++
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/builtin-fast-export.c b/builtin-fast-export.c
index 7d5d57a..0af4e6f 100644
--- a/builtin-fast-export.c
+++ b/builtin-fast-export.c
@@ -297,10 +297,14 @@ static void handle_tag(const char *name, struct tag *tag)
message_size = strlen(message);
}
tagger = memmem(buf, message ? message - buf : size, "\ntagger ", 8);
- if (!tagger)
- die ("No tagger for tag %s", sha1_to_hex(tag->object.sha1));
- tagger++;
- tagger_end = strchrnul(tagger, '\n');
+ if (!tagger) {
+ warning ("No tagger for tag %s", sha1_to_hex(tag->object.sha1));
+ tagger = "tagger No Tagger <no-tagger> 0 +0000";
+ tagger_end = tagger + strlen(tagger);
+ } else {
+ tagger++;
+ tagger_end = strchrnul(tagger, '\n');
+ }
/* handle signed tags */
if (message) {
diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh
index 2057435..2312d7a 100755
--- a/t/t9301-fast-export.sh
+++ b/t/t9301-fast-export.sh
@@ -239,4 +239,19 @@ test_expect_success 'fast-export | fast-import when master is tagged' '
'
+cat > tag-content << EOF
+object $(git rev-parse HEAD)
+type commit
+tag rosten
+EOF
+
+test_expect_success 'cope with tagger-less tags' '
+
+ TAG=$(git hash-object -t tag -w tag-content) &&
+ git update-ref refs/tags/sonnenschein $TAG &&
+ git fast-export -C -C --signed-tags=strip --all > output &&
+ test $(grep -c "^tag " output) = 4
+
+'
+
test_done
--
1.6.1.rc3.368.g63acc
^ permalink raw reply related
* Re: [PATCH] Add git-edit-index.perl
From: Johannes Schindelin @ 2008-12-18 19:47 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Jeff King, Neil Roberts, git
In-Reply-To: <20081218163654.GR5691@genesis.frugalware.org>
Hi,
On Thu, 18 Dec 2008, Miklos Vajna wrote:
> On Thu, Dec 18, 2008 at 05:24:00PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> > On Thu, 18 Dec 2008, Jeff King wrote:
> >
> > > It _would_ be a nicer workflow to say "I don't want these changes
> > > yet" and selectively put them elsewhere, test what's in the working
> > > tree, commit, and then grab some more changes from your stash. But
> > > we don't have interactive stashing and unstashing yet, which would
> > > be required for that.
> >
> > git stash -i... Yes, I'd like that!
>
> Or git checkout -i?
Frankly, I need to move changes away much more often. Plus, you could
have what you wished for with a "git checkout -- <path> && git stash -i".
It's just that you would move out the changes you would not want yet.
Ciao,
Dscho
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox