* Re: [PATCH 1/3] git-submodule: allow submodule name and path to differ
From: Junio C Hamano @ 2007-06-10 8:23 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Lars Hjemli, Sven Verdoolaege, git
In-Reply-To: <Pine.LNX.4.64.0706100820560.4059@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Sat, 9 Jun 2007, Junio C Hamano wrote:
>
>> Lars Hjemli <hjemli@gmail.com> writes:
>>
>> > This teaches git-submodule to check module.*.path when looking for the
>> > config for a submodule path. If no match is found it falls back to the
>> > current behaviour (module.$path).
>>
>> I have a feeling that it might be much less troublesome in the longer
>> term to admit that module.$path was a mistake and support only one
>> format; wouldn't trying to support both leave ambiguity and confusion?
>
> Just my 2cents: git-submodule is not yet in any released version. So let's
> fix things early. In our world, it's not like you lose face when you have
> to admit mistakes. (Instead, you lose face when you refuse to fix them.)
> Ah, if only politics learnt from our world...
Well, I completely agree with what you said up to "So let's fix
things early.", but when I re-read what I wrote, I realize that
my wording was bad --- I did not mean to drive the discussion in
that direction.
It was not Lars's *mistake* to admit to begin with. His was one
valid design that was consistent within his patch series. The
thing is that there just was a better alternative suggested
later; it does not make the first iteration a mistake in any
way.
So, Lars, my apologies if I offended you -- I did not mean it
that way.
^ permalink raw reply
* Re: [PATCH 07/21] Copy the remaining differences from verify_tag() to parse_tag_buffer_internal()
From: Johannes Schindelin @ 2007-06-10 8:22 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Alex Riesen, Junio C Hamano
In-Reply-To: <200706092339.12821.johan@herland.net>
Hi,
On Sat, 9 Jun 2007, Johan Herland wrote:
> I'm probably splitting this up into too small pieces, since I keep
> getting comments that fail to see the overall picture of what I'm trying
> to do, [...]
Maybe I said that your patch was too large. But then, I said something
much more important: hard-to-review.
These small patches, split in a manner making it even more difficult to
understand what you want to accomplish, do not help.
Yes, you should make small patches. Even small patch series. But in such a
fashion that a reviewer can see that it is a good patch[*1*]. Just lean
back, look at your patches, and ask yourself how you would have reacted if
you had reviewed them.
Ciao,
Dscho
*1* A good patch follows the immortal words of Saint Exupery: A designer
knows he has achieved perfection not when there is nothing left to add,
but when there is nothing left to take away.
^ permalink raw reply
* Re: Refactoring the tag object; Introducing soft references (softrefs); Git 'notes' (take 2)
From: Pierre Habouzit @ 2007-06-10 8:29 UTC (permalink / raw)
To: Johan Herland; +Cc: Steven Grimm, git, Junio C Hamano
In-Reply-To: <200706100116.46062.johan@herland.net>
[-- Attachment #1: Type: text/plain, Size: 1534 bytes --]
On Sun, Jun 10, 2007 at 01:16:45AM +0200, Johan Herland wrote:
> On Sunday 10 June 2007, Steven Grimm wrote:
> > Being able to specify relationships between commits after the fact seems
> > like a very useful facility.
> >
> > Does it make sense to have type information to record what the
> > relationship between two objects means? Without that, it seems like
> > it'll be hard to build much of a tool set on top of this feature, since
> > no two tools that made use of it could unambiguously query just their
> > own softrefs.
>
> Actually MadCoder/Pierre had a similar idea on IRC. He wanted to separate
> softrefs into namespaces, so that softrefs for tags could live in a
> different place than softrefs associated with his "gits" bug tracker.
>
> I haven't thought very much about this, but it's certainly possible to do
> something like this. What do the rest of y'all think?
Well, if we're two with the same idea, it's a good one, no ? :)
In fact, the namespace idea like I told you on IRC isn't _that_
brilliant. But I'm sure recording a softref with:
<from_sha> <to_sha> <token>
token would help classify the softref. And I'm sure we'll end up with:
<from_sha> <to_sha> <token> <flags>
with the flags to say what behaviour (e.g.) the reachability resolver
should have wrt that link ?
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] Change softrefs file format from text (82 bytes per entry) to binary (40 bytes per entry)
From: Junio C Hamano @ 2007-06-10 8:30 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Johan Herland, git, Linus Torvalds
In-Reply-To: <Pine.LNX.4.64.0706100900420.4059@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Sat, 9 Jun 2007, Johan Herland wrote:
>
>> The text-based softrefs file format uses 82 bytes per entry (40 bytes
>> from_sha1 in hex, 1 byte SP, 40 bytes to_sha1 in hex, 1 byte LF).
>>
>> The binary softrefs file format uses 40 bytes per entry (20 bytes
>> from_sha1, 20 bytes to_sha1).
>>
>> Moving to a binary format increases performance slightly, but sacrifices
>> easy readability of the softrefs files.
>
> It is bad style to introduce one type, and then change it to another in a
> backwards-incompatible way. Either you make it backwards compatible, or
> you start with the second format, never even mentioning that you had
> another format.
While I agree with that in principle, I think you are being a
bit too harsh to a set of patches that shows possible
alternatives for an idea that is not even in any unreleased
version of git.
Got out of the wrong side of bed this morning?
^ permalink raw reply
* Re: [RFC] git integrated bugtracking
From: Martin Langhoff @ 2007-06-10 8:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4plgb6t6.fsf@assigned-by-dhcp.cox.net>
On 6/10/07, Junio C Hamano <gitster@pobox.com> wrote:
> > Adding git & gitweb support to traq, bugzilla, mantis, gforge, etc is
> > what is going to make the difference. Most of those have already the
> > ability to "link" to one or more commits -- after the commits are done
> > and in GIT.
>
> You are a brave person to say this (no sarcasm --- I wish I
> were, too).
>
> On one hand, I very much applaud and appreciate your comment for
> injecting sanity to the discussion. On the other hand, if Linus
> had such an attitude, we might not be hacking on git right now.
Thanks for the reply - sorry for being a bit of a stirrer ;-) Yes, I
do appreciate the irony and I'd have never started git. If anyone
really wants to do their own bugtracker... be like Linus not like me
(a mere follower) ;-)
> After looking at the above existing alternatives, some brave
> soul might decide and say, "Hey, I can write something better in
> 2 weeks" ;-).
Definitely. But that takes a deeper look into the above alternatives,
and probably a good perspective. I hope noone takes offence if I say
that -- as a user of several bugtrackers over ~10 years -- I haven't
yet read anything in this thread that is exciting.
And "it's closely integrated with git" can actually be a misfeature.
Cool if it's what gets you going, but not enough for world domination
;-)
> All of your examples are going from a single bug to commits, but
> from a release person's point of view, you are never interested
> in a single bug, just like a top-level maintainer is never
> interested in a single file. A release person would want to go
> in the reverse direction: from a commit range to a set of bugs.
That's right. It's not that hard to correlate commits-in-the-changelog
and ask the bugtracker to produce a "tracker changelog" showing all
the bugs/tasks that link to the commits (with that liberal view of
"links to" discussed before). Formatting of the tracker changelog _is_
tricky but that's due to the ambiguities in those relationships.
(In Moodle I use JIRA, a non-FOSS tracker, and it does exactly that. I
think it has some notion of CVS branches, and where tags sit, so you
can ask for a report of which bugs are fixed in the release you are
preparing on branch X).
> What bugs were fixed and what regressions were introduced during
> this release cycle. While embedded ticket numbers in commit log
> messages would certainly help, a change made to fix a particular
> bug may fix another as its side effect, and the develeoper who
> did the change may not know about the latter when the commit log
> message is written.
I agree it's useful, but I don't think it has any benefit having it in
the SCM _at all_. Having them in the BT is a lot more flexible -- and
the fact that git has stable commit IDs makes it easier to integrate
in a BT; as the BT can spot that the commit fixing bug 123 is now
merged into head ZZ as well as head YYY.
So the BT holds "external tags" to your commits. Lots of them. With a
ton of data. And that's great -- in fact there's no stopping it -- the
stability of SHA1s means that the whole internet holds tags to your
repo. Every time a SHA1 (or output of git-describe) is mentioned in a
mailing list, wiki or forum, there's a tag.
Now, to rule the world, BTs gain a lot more from being able to
integrate with different SCMs, automated test systems (like
tinderbox), MTAs (debbugs), wikis (traq), stats tracking for PHBs
(bugzilla), etc. So loose coupling wins here, and git's SHA1s are
great for that.
And at git's end we can get the smooth integration using/abusing that
loose coupling strategy. So if git-log/gitk/qgit grow some hooks that
can be used to perform a lookup... I can think of a perl script that
gets events for each SHA1 that git-rev-list produces and returns one
bug number, url and title (or more than one) that then git-log / gitk
/ qgit / annotate can use to further "decorate" the commit info in a
similar style to what gitk is doing now with head names in each
commit.
Would that fit your vision of a nicely integrated tracker?
martin (who's now thinking how to craft a proof of concept)
^ permalink raw reply
* Re: [PATCH 1/3] git-submodule: allow submodule name and path to differ
From: Lars Hjemli @ 2007-06-10 8:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Sven Verdoolaege, git
In-Reply-To: <7vbqfod57b.fsf@assigned-by-dhcp.cox.net>
On 6/10/07, Junio C Hamano <gitster@pobox.com> wrote:
> Lars Hjemli <hjemli@gmail.com> writes:
>
> > This teaches git-submodule to check module.*.path when looking for the
> > config for a submodule path. If no match is found it falls back to the
> > current behaviour (module.$path).
>
> I have a feeling that it might be much less troublesome in the
> longer term to admit that module.$path was a mistake and support
> only one format;
Hmm, what the patch does should match what is described in
http://article.gmane.org/gmane.comp.version-control.git/48879
except that I've kept [module] instead of renaming it to [submodule].
But my wording ("module.$path") might be disconcerting :)
Fwiw: I do admit that the [path "path"] construct was a mistake.
--
larsh
^ permalink raw reply
* Re: [RFC] git integrated bugtracking
From: Jan Hudec @ 2007-06-10 8:37 UTC (permalink / raw)
To: Martin Langhoff; +Cc: git
In-Reply-To: <46a038f90706092359i43a6e834rc096e53a28fbee51@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3772 bytes --]
On Sun, Jun 10, 2007 at 18:59:13 +1200, Martin Langhoff wrote:
> On 6/10/07, Pierre Habouzit <madcoder@debian.org> wrote:
> > FWIW I've begun to work on this (for real). I've called the tool
> >"grit". You can follow the developpement on:
> >
> > * gitweb: http://git.madism.org/?p=grit.git;a=summary
> > * git: git://git.madism.org/grit.git/
>
> Call me a fool, but writing a <new> bugtracker looks like a
> boil-the-oceans scheme.
I don't know about any *distributed* bug tracker, which is the point here.
We have several distributed version control tools, but no other distributed
tools for the other tasks in configuration management.
> Adding git & gitweb support to traq, bugzilla, mantis, gforge, etc is
> what is going to make the difference.
It would certainly be useful. But the problem with these bug trackers is,
that they don't go all that well with how many hackers work. And the less
they fit with distributed workflow.
- The web interface is usually not a good match for the problem. Email
interface is better in many respects, but it still does not cut it.
- You can't really use the ability of version control to work disconnected,
when you don't also have the bug information.
- Distributed version control is designed to decrease the workload of the
central maintainer(s) while keeping him in control. But with centralized
bug tracking he still has a lot of work with that that the mob can't help
him with. It helps if the bug tracker can close bugs based on something in
commit-log, but you also need to see what it is that will be closed, which
requires integrating the bug tracker into the version control.
The other thing is, that a good bug tracker also needs to be integrated with
the *mailing list*. The major part of bug tracking is discussing those bugs
and discussions usually take place on mailing-lists (and on irc --
integrating that as well would be nice too).
I've actually already seen some attempts at something like this project. The
first one was for GNU Arch. Instead of using a bug-tracker, there was
a branch where each bug had it's mbox with all the relevant data. This mbox
was moved between directories depending on it's state. AFAIR there was no
tool to maintain it, just a description of the workflow.
(Tom Lord called it "the game". See eg.
http://www.dasbistro.com/~sam/mail/tom-lords-game)
The other attempt is obviously the Bugs Everywhere thing.
A related thing is Bazaar's "Bundle Buggy". It tracks patch submission rather
than bugs, but it is roughly what I meant by the "mailing list integration"
above. It "reads" the mailing list, watching for anything labeled [PATCH] or
[BUNDLE] with patch or bundle (bundle is bazaar's extended patch) attached.
It also watches for replies with review (review outcome is stated with "-1"
(veto), "-0", "+0" and "+1" (ok)) and with further patches/bundles (updated
versions of the patch).
> [...]
>
> And definitely, if you use git as an alibi to write a new bugtracker,
> don't use the "works only with git" as a feature. It should work with
> as many SCMs as possible.
If it uses git as it's database, which it probably will, it will definitely
require git. Now it should be possible to have the code in different version
control, but it would be integration with things like git format-patch, that
will really make it cool stuff -- and that won't work with other version
control out of the box.
> OTOH, that's just me, I'm lazy and like to work on already-successful
> projects that are 99% there for my needs (and where I can add that
> 1%).
Yes. But for many people current bug tracking tools do NOT work 99%.
--
Jan 'Bulb' Hudec <bulb@ucw.cz>
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] Robustify rebase test: check patches are reapplied.
From: Karl Hasselström @ 2007-06-10 8:40 UTC (permalink / raw)
To: Yann Dirson; +Cc: Catalin Marinas, git
In-Reply-To: <20070606220142.18635.3961.stgit@gandelf.nowhere.earth>
On 2007-06-07 00:01:42 +0200, Yann Dirson wrote:
> + test `stg applied | wc -l` = 1
Unimportant note: I believe "stg applied" has a -c (count) flag for
precisely this purpose.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [PATCH 10/21] Free mktag's buffer before dying
From: Johannes Schindelin @ 2007-06-10 8:38 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano
In-Reply-To: <200706090216.41856.johan@herland.net>
Hi,
On Sat, 9 Jun 2007, Johan Herland wrote:
How does this:
> - usage("git-mktag < signaturefile");
> + usage("git-mktag < tag_data_file");
and this:
> -
relate with your commit message?
You might understand that people _could_ get the impression that the
patches were not very carefully crafted. Or even worse, the impression,
that it was tried to slip some changes by, with a totally unrelated
"official" purpose. (Again, reminds me of politics: it's like
slipping an intrusive privacy law into an agriculture related law at the
11th hour, just before Christmas, when people do not really pay
attention.)
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 11/21] Rewrite error messages; fix up line lengths
From: Johannes Schindelin @ 2007-06-10 8:38 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano
In-Reply-To: <200706090217.17795.johan@herland.net>
Hi,
On Sat, 9 Jun 2007, Johan Herland wrote:
> Also update selftests to reflect new error messages.
Had you split the patch conceptually, the changes to the tests would have
been included where appropriate.
I consider a patch _breaking_ a test case, with a follow up patch to the
test case, a _bug_.
And I consider a change in error messages not good, _unless_ the changes
have a real value in practice. Which I maintain they do not, in this
patch-series' case.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/3] git-submodule: allow submodule name and path to differ
From: Lars Hjemli @ 2007-06-10 8:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Sven Verdoolaege, git
In-Reply-To: <7vps448bgx.fsf@assigned-by-dhcp.cox.net>
On 6/10/07, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Sat, 9 Jun 2007, Junio C Hamano wrote:
> >
> >> Lars Hjemli <hjemli@gmail.com> writes:
> >>
> >> > This teaches git-submodule to check module.*.path when looking for the
> >> > config for a submodule path. If no match is found it falls back to the
> >> > current behaviour (module.$path).
> >>
> >> I have a feeling that it might be much less troublesome in the longer
> >> term to admit that module.$path was a mistake and support only one
> >> format; wouldn't trying to support both leave ambiguity and confusion?
> >
> > Just my 2cents: git-submodule is not yet in any released version. So let's
> > fix things early. In our world, it's not like you lose face when you have
> > to admit mistakes. (Instead, you lose face when you refuse to fix them.)
> > Ah, if only politics learnt from our world...
>
> Well, I completely agree with what you said up to "So let's fix
> things early.", but when I re-read what I wrote, I realize that
> my wording was bad --- I did not mean to drive the discussion in
> that direction.
>
> It was not Lars's *mistake* to admit to begin with. His was one
> valid design that was consistent within his patch series. The
> thing is that there just was a better alternative suggested
> later; it does not make the first iteration a mistake in any
> way.
>
> So, Lars, my apologies if I offended you -- I did not mean it
> that way.
No need for apologies, no offence taken :)
--
larsh
^ permalink raw reply
* Re: [PATCH 12/21] Use prefixcmp() instead of memcmp() for cleaner code with less magic numbers
From: Johannes Schindelin @ 2007-06-10 8:41 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano
In-Reply-To: <200706090217.49818.johan@herland.net>
Hi,
On Sat, 9 Jun 2007, Johan Herland wrote:
>
> - if (memcmp(data, "object ", 7))
> + if (prefixcmp(data, "object "))
FWIW I think that _these_ changes are actually somewhat worth it. And they
should have come _instead_ of moving the order of the memcmp() around
(you know which patch that was).
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 13/21] Collect skipping of header field names and calculation of line lengths in one place
From: Johannes Schindelin @ 2007-06-10 8:45 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano
In-Reply-To: <200706090218.16281.johan@herland.net>
Hi,
On Sat, 9 Jun 2007, Johan Herland wrote:
> + /*
> + * Advance header field pointers past their initial identifier.
> + * Calculate lengths of header fields.
> + */
> + type_line += strlen("type ");
> + type_len = tag_line - type_line - 1;
> + tag_line += strlen("tag ");
> + tag_len = tagger_line - tag_line - 1;
> +
> /* Get the actual type */
> - type_len = tag_line - type_line - strlen("type \n");
> if (type_len >= sizeof(type))
> return error("Tag object (@ char " PD_FMT "): "
> - "Type too long", type_line + 5 - data);
> - memcpy(type, type_line + 5, type_len);
> + "Type too long", type_line - data);
> + memcpy(type, type_line, type_len);
This change does not clarify anything. It is exactly as confusing as
before.
> - for (i = 4;;) {
> + for (i = 0;;) {
I know you introduced this in another patch. This is an ugly construct. If
you want people to review your patches, you might as well put in the
effort to make the reviewing more pleasant.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC] git integrated bugtracking
From: Jan Hudec @ 2007-06-10 8:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Martin Langhoff, git
In-Reply-To: <7v4plgb6t6.fsf@assigned-by-dhcp.cox.net>
[-- Attachment #1: Type: text/plain, Size: 1835 bytes --]
On Sun, Jun 10, 2007 at 00:35:33 -0700, Junio C Hamano wrote:
> "Martin Langhoff" <martin.langhoff@gmail.com> writes:
> > So you can tell your bugtracker
> > - which commit fixed it -- usually auto-linked if you include the
> > bugnumber in the commit message
> > - which commit added the test -- auto linked as above
> > - which commit introduced the bug -- if such thing exists and someone
> > digs it up
>
> All of your examples are going from a single bug to commits, but
> from a release person's point of view, you are never interested
> in a single bug, just like a top-level maintainer is never
> interested in a single file. A release person would want to go
> in the reverse direction: from a commit range to a set of bugs.
> What bugs were fixed and what regressions were introduced during
> this release cycle. While embedded ticket numbers in commit log
> messages would certainly help, a change made to fix a particular
> bug may fix another as its side effect, and the develeoper who
> did the change may not know about the latter when the commit log
> message is written.
It would be really useful to have a tool, that could link a bug report to
a test case demonstrating it and reporting whenever output of that test case
changes. This would make it much easier for developer to see which bugs he
might have fixed when doing a refactoring.
It should probably report not just unexpected success, but also change to the
error output, because the test case does not have to be 100% correct. That is
if you have a test case that prints:
testFoobar FAIL "foo" != "Bar"
and than starts to say:
testFoobar FAIL "foo" != "Foo"
the problem is probably fixed, but the test still fails because of minor
error in the expected output string.
--
Jan 'Bulb' Hudec <bulb@ucw.cz>
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH 14/21] Add proper parsing of "tagger" line, but only when thorough_verify is set
From: Johannes Schindelin @ 2007-06-10 8:52 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano
In-Reply-To: <200706090218.41941.johan@herland.net>
Hi,
On Sat, 9 Jun 2007, Johan Herland wrote:
> Be explicit about the fact that the "tagger" line is now considered a
> mandatory part of the tag object. There are however old tags (from
> before July 2005) that don't have a "tagger" line. We therefore consider
> the "tagger" line _optional_ when parsing tags without thorough_verify
> set.
No. The "before July 2005" part is _not_ the reason that we consider this
line optional.
The fact that it is bad to fail on a fetch, just because you happen to
have an invalid tag in your repository, is a good reason not to.
The fact that it is bad to fail on a git branch, just because you happen
to have an invalid tag in your repository, is a good reason not to.
The fact that it is bad to fail on an fsck, just because you happen to
have an invalid tag in your repository, is a good reason not to.
And yes, if I remember correctly, your original patch did exactly that.
The paradigm to follow is: fail gracefully. I could have an invalid
_commit_ in my repository, and would still want _every_ Git operation to
succeed, _as long_ as it does not touch that bad object.
And I damned well want git-fsck to not crash, just because some
assumptions are made.
Since this is a fundamental critique on your patch series, I will do the
detailed review on _this_ patch in another mail.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC] git integrated bugtracking
From: Martin Langhoff @ 2007-06-10 8:55 UTC (permalink / raw)
To: Jan Hudec; +Cc: git
In-Reply-To: <20070610083754.GC4084@efreet.light.src>
On 6/10/07, Jan Hudec <bulb@ucw.cz> wrote:
> I don't know about any *distributed* bug tracker, which is the point here.
As an end user, I suspect I _don't_ want to have to report a bug in a
distributed bug tracker ;-) In that space, the Malone guys (canonical)
have their fingers on one of the most serious issues, and perhaps it's
interesting to see what they've done there. It's really useful, even
if I don't think I want to have to maintain it :-/
> We have several distributed version control tools, but no other distributed
> tools for the other tasks in configuration management.
Bugtrackers are co-owned by developers, users and (where they exist)
project managers.
> - The web interface is usually not a good match for the problem. Email
> interface is better in many respects, but it still does not cut it.
I agree. I love/hate debbugs too.
> - You can't really use the ability of version control to work disconnected,
> when you don't also have the bug information.
A cache fixes the reading part - see my other post, and imagine being
able to have a local sqlite cache of the BT key data indexed by
referenced SHA1, showing up with your commits in gitk.
The write part is solved (in part) by committing to git the fix -- if
you mentionm the bug ID, the central BT will pick it up when your
commit appears in the branches/repos that the BT can see.
For "just adding a comment", the write part is solved by the "email"
interface, like with debbugs.
> - Distributed version control is designed to decrease the workload of the
> central maintainer(s) while keeping him in control. But with centralized
And to provide a single place for users to report a problem and track
its status.
> If it uses git as it's database, which it probably will,
Well - hmmm. Git's database is great at tracking _content identity_.
But a bug's content identity (description+comments+status) changes all
the time. I don't think it's naturally a good match.
Perhaps it makes sense to mix git's storage model with something else...?
> Yes. But for many people current bug tracking tools do NOT work 99%.
Hmmm. I agree in that "does not work disconnected" is a big issue with
web tools, but debbugs works disconnected, and is good. Git's
bugtracker (git@vger) works disconnected too ;-) And googlegears might
help the rest of us. Is there any other problem with current BTs?
cheers,
martin
^ permalink raw reply
* Re: [PATCH 2/3] Add gitmodules(5)
From: Lars Hjemli @ 2007-06-10 8:58 UTC (permalink / raw)
To: Frank Lichtenheld
Cc: Junio C Hamano, Johannes Schindelin, Sven Verdoolaege, git
In-Reply-To: <20070610002802.GD31707@planck.djpig.de>
On 6/10/07, Frank Lichtenheld <frank@lichtenheld.de> wrote:
> The unofficial git documentation nitpicker at work ;)
That's good, keep it up :)
>
> On Sat, Jun 09, 2007 at 11:38:51PM +0200, Lars Hjemli wrote:
> > +The `.gitmodules` file, located in the top-level directory of a
> > +gitlink:git[7] working tree, is a text file with a layout matching the
>
> That link seems superfluous to me.
Yeah, what I really wanted here was a link to the definition of "git
working tree". I'll drop it.
> I would have used "syntax" instead of "layout".
Agreed
>
> > +requirements of gitlink:git-config[1].
> > +
> > +The file consists of sections named `module`, divided into one subsection
> > +per submodule. The subsections are named with the logical name of the
> > +submodule it describes.
>
> "sections named module" sounds confusing to me. Why are there multiple
> sections named module? (for the record: I know what you mean, I just
> don't know if it couldn't be said simpler)
> Maybe better "subsections of section `module`, one per submodule"?
> Hmm, sounds ugly too.
Good documentation is hard, so I'll work on it some more...
>
> > +Each submodule can contain the following keys.
> > +
> > +module.$name.path::
> > + Define a path, relative to the top-level directory of the git
> > + working tree, where the submodule is expected to be checked out.
> > +
> > +module.$name.url::
> > + Define a url from where the submodule repository can be cloned.
>
> For .path a "Defaults to name of submodule" probably wouldn't hurt.
True. But there might be some issues with this rule, so I'll leave it
as is for now.
>
> For the sake of documentation consistency I would suggest
> module.<name>.path. You can compare the output of
> $ grep "\.<[a-z]" Documentation/*.txt
> with
> $ grep "\.\$[a-z]" Documentation/*.txt
> to see what I mean.
That was very descriptive, thanks!
>
> > + [module 'libfoo']
> > + path = include/foo
> > + url = git://example1.com/git/libfoo.git
> > +
> > + [module 'libbar']
> > + url = git://example2.com/pub/git/libbar.git
>
> This would actually be a syntax error in a git config file
> (subsection names can be enclosed in "" but not '').
Shame on me for drinking while documenting ;-)
Thanks for the review, I'll try to send a fixed-up patch later today.
--
larsh
^ permalink raw reply
* Re: [PATCH 14/21] Add proper parsing of "tagger" line, but only when thorough_verify is set
From: Johannes Schindelin @ 2007-06-10 8:58 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano
In-Reply-To: <200706090218.41941.johan@herland.net>
Hi,
On Sat, 9 Jun 2007, Johan Herland wrote:
> - const char *type_line, *tag_line, *tagger_line;
> - unsigned long type_len, tag_len;
> + const char *type_line, *tag_line, *tagger_line;
> + unsigned long type_len, tag_len, tagger_len;
> + const char *header_end;
This is ugly. Really ugly. Besides, it breaks the minimal patch paradigm.
> @@ -81,7 +82,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
> if (size < 64)
> return error("Tag object failed preliminary size check");
>
> - /* Verify object line */
> + /* Verify mandatory object line */
> if (prefixcmp(data, "object "))
> return error("Tag object (@ char 0): "
> "Does not start with \"object \"");
Hmph. I think everybody reading C code understands that this is mandatory.
I even think that the comment is useless. It is sort of a
code-in-human-language duplicated code.
> - /* Verify the tagger line */
> + /*
> + * Verify mandatory tagger line, but only when we're checking
> + * thoroughly, i.e. on inserting a new tag, and on fsck.
> + * There are existing tag objects without a tagger line (most
> + * notably the "v0.99" tag in the main git repo), and we don't
> + * want to fail parsing on these.
> + */
I maintain that even with thorough checking, it is _wrong_ to error on a
missing tagger. Since we have to deal with tagger-less tags _anyway_, and
since it is not like you could really do something about it (the tag is
immutable), you should go with a warning.
> - * Calculate lengths of header fields.
> + * Calculate lengths of header fields (0 for fields that are not given).
Does that really make sense? You effectively treat a missing field as if
it were empty. IMHO that is wrong. Besides, this
> + tagger_len = header_end > tagger_line ?
> + (header_end - tagger_line) - 1 : 0;
is really ugly, what with in-line spaces, _and_ special casing.
> - /* Verify the tagger line */
> + /* Verify tagger line */
Hmpf.
Ciao,
Dscho
^ permalink raw reply
* Re: Comment on weak refs
From: Pierre Habouzit @ 2007-06-10 9:03 UTC (permalink / raw)
To: Johan Herland; +Cc: Junio C Hamano, git, Linus Torvalds
In-Reply-To: <200706100325.32846.johan@herland.net>
[-- Attachment #1: Type: text/plain, Size: 4962 bytes --]
On Sun, Jun 10, 2007 at 03:25:32AM +0200, Johan Herland wrote:
> On Sunday 10 June 2007, Junio C Hamano wrote:
> > It gets worse when you actually start using softrefs. I do not
> > think you would have a limited set of softrefs, such as
> > "reverse-tag-lookup-softref", "bug-tracker-softref". For
> > example, a typical bug tracking sheet may look like this:
> >
> > - Hey I found a bug, you can reproduce like so... I am
> > testing commit _A_.
> > - It appears that commit _B_ introduced it; it does not
> > reproduce with anything older.
> > - Here is a patch to fix it; please try.
> > - Oh, it seems to fix. Committed as _C_.
> > - No, it does not work for me, and the approach to fix is
> > wrong; reopening the bug.
> > - Ok, take 2 with different approach. Committed as _D_.
> > please try.
> > - Thanks, it finally fixes it. Closing the bug.
> >
> > The bug will be associated with commits A, B, C and D. The
> > questions maintainers would want to ask are:
> >
> > - What caused this bug?
> > - Which versions (or earlier) have this bug?
> > - Which versions (or later) have proper fix?
> > - What alternate approaches were taken to fix this bug?
> > - In this upcoming release, which bugs have been fixed?
> > - What bugs are still open after this release?
> >
> > Depending on what you want to find out, you would need to ask
> > which commits are related to this bug tracking sheet object, and
> > the answer has to be different. Some "softref" relation should
> > extend to its ancestry (when "this fixes" is attached to a
> > commit, its children ought to inherit that property), some
> > shouldn't ("this is what broke it" should not propagate to its
> > parent nor child).
>
> We're getting a little ahead of ourselves, aren't we? IMHO, it would be up
> to the bug system to determine which (and how many) connections to make
> between the bug reports and the commits (or even if softrefs would be the
> correct mechanism for these connections at all). We shouldn't necessarily
> base the softrefs design on how we imagine a hypothetical bug system to
> work. But Pierre might have something to say on how he would want to use
> softrefs, and his system is hopefully _less_ hypothetical. :)
To be fair, I'm still struggling with the storage backend yet, trying
to make things fast enough (My current import rate of mails is 10 per
second, wich is not that brilliant I guess), and also to design some
simple things like "answering" to a bug.
For now, my design is the following, I've a 'bts' branch where the
bugs reports (plain mailboxes) go. Grit is able to manage as many branch
the user wants, bts is just the default name for it. Then, for a bts
branch, you have $GIT_DIR/grit/<branch>.index and
$GIT_DIR/grit/<branch>/. The former is the index of the tip of the bts
branch, and the latter contains some bits of the tip of the branch
checkouted (can be seen as some kind of cache, useful to run mutt -f on
a mbox e.g.).
Bugs have the sha id of the hash of the first imported mail, and are
put in sha[:2]/sha[2:] files, à la .git/objects/. I also should have a
second file with annotations about the bug, format not really clear for
now, as "one file per bug" could be quite inefficient. OTOH if I mix too
many bugs in the same file, the merge risk is bigger (but I suppose I
could use a specific merge strategy on this).
Here is the sole non hypothetical thing yet. My plans then was to use
"links" (softlinks or not, I'm speaking generically, I hope softrefs
will match my needs, I don't know yet) between specific commits, and
bugs. Links would somehow carry information on wether this is an
"opening" tag (like: this bug is present starting at that commit), an
informationnal tag (like: this commit helps fixing that bug, but is not
enough), or a closing/fixing tag (like: this commit fixes it). A fourth
kind may be also used aka a not-found tag (like: well this commit does
not fixes the bug, but for sure it's not there anymore at that commit).
Though, softlinks do not need to "carry" the information for real,
they just need to be linked somehow to the bug, bug that would have the
annotations for those softlinks in them.
What is somehow flawed for me, is that when someone "answers" to the
bug or changes a bit of information about it, it generate a "new"
commit, and I would need to move the softlinks to the new commit object
it generated to shorten the path and go directly to the last version of
the bug status file.
So to be of use for me, yes, I guess I would really like the
versionning of softlinks. If I use them at all, I don't know yet.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH 06/21] Refactor tag name verification loop to use index 'i' instead of incrementing pointer 'tag_line'
From: Johannes Schindelin @ 2007-06-10 9:01 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano
In-Reply-To: <200706090215.05318.johan@herland.net>
Hi,
On Sat, 9 Jun 2007, Johan Herland wrote:
> diff --git a/mktag.c b/mktag.c
> index 4dbefab..2e70504 100644
> --- a/mktag.c
> +++ b/mktag.c
> @@ -81,19 +81,22 @@ static int verify_tag(char *data, unsigned long size)
> memcpy(type, type_line + 5, type_len);
> type[type_len] = '\0';
>
> - /* Verify that the object matches */
> - if (verify_object(sha1, type))
> - return error("char%d: could not verify object %s", 7, sha1_to_hex(sha1));
> -
> - /* Verify the tag-name: we don't allow control characters or spaces in it */
> - tag_line += 4;
> - for (;;) {
> - unsigned char c = *tag_line++;
> - if (c == '\n')
> - break;
> - if (c > ' ')
> - continue;
> - return error("char" PD_FMT ": could not verify tag name", tag_line - data);
> + {
> + unsigned long i;
Do you realize that half of your diff consists of reindenting, just
because you introduced this ugly construct, instead of being a good boy
and put the declarations where they belong -- at the beginning of the
function (or if it exists, block)?
> + /* Verify the tag-name: we don't allow control characters or spaces in it */
> + for (i = 4;;) {
Yes, you can write this construct. That does not change the fact that it
gives me eye cancer.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 15/21] Make tag names (i.e. the tag object's "tag" line) optional
From: Johannes Schindelin @ 2007-06-10 9:07 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano
In-Reply-To: <200706090219.12011.johan@herland.net>
Hi,
On Sat, 9 Jun 2007, Johan Herland wrote:
> The tag line is now optional. If not given in the tag object data, it
> defaults to the empty string ("") in the parsed tag object.
>
> The patch also adds a change to git-show; when asked to display a tag
> object with no name (missing "tag" header), we will show the tag's sha1
> instead of an empty string.
>
> Finally the patch includes some tweaks to the selftests to make them
> work with optional tag names.
If you don't actually _test_ missing tag names, you might just as well
leave the tests alone.
> - t->tag,
> + *(t->tag) ? t->tag : name,
This is misleading. What you wanted to say is t->tag[0] == '\0', or
*(t->tag) == '\0'.
As you wrote it, you have to think a couple of times why it is okay to
dereference t->tag, to check if you say t->tag.
Besides, it breaks if you _do_ have an empty tag. In that case, I _want_
to see that it is actually empty, and _not_ the SHA1 substituted for it.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] git-svn: use git-log rather than rev-list | xargs cat-file
From: Sam Vilain @ 2007-06-10 9:00 UTC (permalink / raw)
To: Eric Wong; +Cc: git
This saves a bit of time when rebuilding the git-svn index.
Signed-off-by: Sam Vilain <sam@vilain.net>
---
git-svn.perl | 30 +++++++++++++++++++-----------
1 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index e350061..610563c 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -802,10 +802,15 @@ sub cmt_metadata {
sub working_head_info {
my ($head, $refs) = @_;
- my ($fh, $ctx) = command_output_pipe('rev-list', $head);
- while (my $hash = <$fh>) {
- chomp($hash);
- my ($url, $rev, $uuid) = cmt_metadata($hash);
+ my ($fh, $ctx) = command_output_pipe('log', $head);
+ my $hash;
+ while (<$fh>) {
+ if ( m{^commit ($::sha1)$} ) {
+ $hash = $1;
+ next;
+ }
+ next unless s{^\s+(git-svn-id:)}{$1};
+ my ($url, $rev, $uuid) = extract_metadata($_);
if (defined $url && defined $rev) {
if (my $gs = Git::SVN->find_by_url($url)) {
my $c = $gs->rev_db_get($rev);
@@ -1964,16 +1969,19 @@ sub rebuild {
return;
}
print "Rebuilding $db_path ...\n";
- my ($rev_list, $ctx) = command_output_pipe("rev-list", $self->refname);
+ my ($log, $ctx) = command_output_pipe("log", $self->refname);
my $latest;
my $full_url = $self->full_url;
remove_username($full_url);
my $svn_uuid;
- while (<$rev_list>) {
- chomp;
- my $c = $_;
- die "Non-SHA1: $c\n" unless $c =~ /^$::sha1$/o;
- my ($url, $rev, $uuid) = ::cmt_metadata($c);
+ my $c;
+ while (<$log>) {
+ if ( m{^commit ($::sha1)$} ) {
+ $c = $1;
+ next;
+ }
+ next unless s{^\s*(git-svn-id:)}{$1};
+ my ($url, $rev, $uuid) = ::extract_metadata($_);
remove_username($url);
# ignore merges (from set-tree)
@@ -1991,7 +1999,7 @@ sub rebuild {
$self->rev_db_set($rev, $c);
print "r$rev = $c\n";
}
- command_close_pipe($rev_list, $ctx);
+ command_close_pipe($log, $ctx);
print "Done rebuilding $db_path\n";
}
--
1.5.0.4.210.gf8a7c-dirty
^ permalink raw reply related
* [PATCH] git-svn: cache max revision in rev_db databases
From: Sam Vilain @ 2007-06-10 9:04 UTC (permalink / raw)
To: Eric Wong; +Cc: git
Cache the maximum revision for each rev_db URL rather than looking it
up each time. This saves a lot of time when rebuilding indexes on a
freshly cloned repository.
Signed-off-by: Sam Vilain <sam@vilain.net>
---
git-svn.perl | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 610563c..c9758a0 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -804,6 +804,7 @@ sub working_head_info {
my ($head, $refs) = @_;
my ($fh, $ctx) = command_output_pipe('log', $head);
my $hash;
+ my %max;
while (<$fh>) {
if ( m{^commit ($::sha1)$} ) {
$hash = $1;
@@ -812,11 +813,14 @@ sub working_head_info {
next unless s{^\s+(git-svn-id:)}{$1};
my ($url, $rev, $uuid) = extract_metadata($_);
if (defined $url && defined $rev) {
+ next if $max{$url} and $max{$url} < $rev;
if (my $gs = Git::SVN->find_by_url($url)) {
my $c = $gs->rev_db_get($rev);
if ($c && $c eq $hash) {
close $fh; # break the pipe
return ($url, $rev, $uuid, $gs);
+ } else {
+ $max{$url} ||= $gs->rev_db_max;
}
}
}
--
1.5.0.4.210.gf8a7c-dirty
^ permalink raw reply related
* Re: fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree.
From: carbonated beverage @ 2007-06-10 8:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Glanzmann, GIT
In-Reply-To: <7vk5uekagd.fsf@assigned-by-dhcp.cox.net>
On Fri, Jun 08, 2007 at 03:27:30AM -0700, Junio C Hamano wrote:
> How did you get your repository into such a state to begin with?
It's kinda odd to see this thread, as I hit this issue today, as well.
If you need another test case, the cvsps git repo shows this too:
ramune/lycaeum:git: git clone http://ydirson.free.fr/soft/git/cvsps.git
<snip checkout messages>
ramune/lycaeum:git: cd cvsps
ramune/lycaeum:cvsps: ls
ramune/lycaeum:cvsps: git checkout -f
warning: You appear to be on a branch yet to be born.
warning: Forcing checkout of HEAD.
fatal: just how do you expect me to merge 0 trees?
ramune/lycaeum:cvsps: cat .git/HEAD
ref: refs/heads/master
ramune/lycaeum:cvsps: ls .git/refs/head/master
ls: .git/refs/head/master: No such file or directory
-- DN
Daniel
^ permalink raw reply
* Re: [PATCH] Change softrefs file format from text (82 bytes per entry) to binary (40 bytes per entry)
From: Johannes Schindelin @ 2007-06-10 9:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johan Herland, git, Linus Torvalds
In-Reply-To: <7vir9w8b4a.fsf@assigned-by-dhcp.cox.net>
Hi,
On Sun, 10 Jun 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Sat, 9 Jun 2007, Johan Herland wrote:
> >
> >> The text-based softrefs file format uses 82 bytes per entry (40 bytes
> >> from_sha1 in hex, 1 byte SP, 40 bytes to_sha1 in hex, 1 byte LF).
> >>
> >> The binary softrefs file format uses 40 bytes per entry (20 bytes
> >> from_sha1, 20 bytes to_sha1).
> >>
> >> Moving to a binary format increases performance slightly, but sacrifices
> >> easy readability of the softrefs files.
> >
> > It is bad style to introduce one type, and then change it to another in a
> > backwards-incompatible way. Either you make it backwards compatible, or
> > you start with the second format, never even mentioning that you had
> > another format.
>
> While I agree with that in principle, I think you are being a
> bit too harsh to a set of patches that shows possible
> alternatives for an idea that is not even in any unreleased
> version of git.
>
> Got out of the wrong side of bed this morning?
Possibly. Except it was not a bed, but an airplane passenger seat.
And it did not help that I totally disagree with the approach: "sorted
list does not do this well, unsorted not that... let's take both!"
So, please take my words with a grain of salt. But I still think that
_what_ I was saying is correct.
Yes, the patch series shows an alternative, but I would have wished for
either a smaller quick-n-dirty proof-of-concept implementation ([RFC]), or
a better thought-through one ([PATCH]).
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