* Re: Comment on weak refs
From: Johan Herland @ 2007-06-10 1:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Linus Torvalds, Pierre Habouzit
In-Reply-To: <7vk5ucd6of.fsf_-_@assigned-by-dhcp.cox.net>
On Sunday 10 June 2007, Junio C Hamano wrote:
> The patch series to look-up and maintain "softref" relationship
> is trivially clean. Although I probably would have many nits to
> pick, I do not think it is _wrong_ in a major way per-se. I
> would not even mind saying that I liked the basic concept, until
> I thought things a bit deeper.
>
> Here are some initial notes I took while reading your patches.
>
>
> Semantics
> ---------
>
> Not all "softref" relationship is equal. "This object is
> referred to by these tags" is one obvious application, and only
> because we already try to follow tags when git-fetch happens
> anyway, it looks natural to make everybody follow such a softref
> relationship.
>
> However, as Pierre Habouzit wants to, we may want to make a bug
> tracking sheet (the details of the implementation of such a bug
> tracking sheet does not matter in this discussion -- it could be
> a blob, a commit, or a tag) refer to commits using this
> mechanism, by pointing at the blob from commits after the fact
> (i.e. "later it was verified that this commit fixes the bug
> described in this bug entry").
>
> Side note: I am assuming a simplest implementation where
> one blob would always capture the latest status of the
> bug. refs/bugs/12127 would point at the latest version
> of bug 12127's tracking sheet. An alternative
> implementation would be to represent each entry of the
> tracking sheet for a single bug as a blob, and have
> multiple blobs associated to a commit on the main
> project, or the other way around, but then you would
> need a way to give order between referers to a single
> referent, which I do not find in your proposed
> "softref".
>
> Most users who want to download and compile the main project do
> not care about bug tracker objects. You would need to have a
> way to describe what kind of relationship a particular softlink
> represents, and adjust the definition of reachability for the
> purposes of traversal of objects.
Yes, I'm starting to see that it's not a good idea to put _all_ softrefs in
one bag.
> 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. :)
But I can see the use of letting the user/porcelain/bugtracker define
classes/namespaces of softrefs (at runtime).
> It is also unclear how relationship "softref" introduces is
> propagated across repositories (not objects the softref binds,
> but the fact that such a binding between two objects exists need
> to be propagated). I would imagine that your assumption is
> simply "to take union". IOW, if you say A refers to B and
> transfer object A to the other side, in addition to transfering
> object B (if the other side does not have it yet), you would
> tell the other side that B is related to A and have the other
> side add that to its set of softrefs. Techinically it is a
> simple and easy to implement semantics, but I suspect that would
> not necessarily be useful in practice. Maybe two people would
> disagree if A and B are related or not.
Yes, I see that different classes of softrefs would have different semantics
for propagation. we could probably try to set up some sane defaults, and
then let users put rules in their configs for how they would want to
propagate the various softrefs classes.
> Maybe you first think A
> and B are related and then later change your mind. Should
> "softref" relationships be versioned?
Intriguing idea. Not immediately sure how we would implement it though...
> Reachability
> ------------
>
> The association brought in between referent and referer by
> softref is "weak", in that referer needs to exist only if
> referent need to be there. This has the following
> consequences.
>
> Fsck/prune/lost-found
> .....................
>
> The current object traversal starts from "known tip objects"
> (i.e. refs, HEAD, index, and reflog entries when not doing
> lost-found) and follows the reachability link embedded in
> referer objects (i.e. tag to tagged object, commit to tree, tree
> to tree and blob). We only need to extend this "reachability"
> with softref. If a referer object refers to another object via
> a softref, we consider referent reachable and we are done.
Agreed.
> This should be reasonably straightforward, except that we
> probably would need to worry about circular references that
> softlink makes possible.
Isn't there a .used flag on objects we could easily check to see if we've
seen an object before, thus preventing us from following the circular
reference?
> push/fetch/rev-list --objects
> .............................
>
> We walk the revision range (object transfers essentially starts
> traversal from the tips of the sender until it meets what the
> receiver already has), enumerating the reachable objects. I
> suspect that adding reachability with softref to this scheme has
> consequences on performance.
>
> Imagine:
>
> A---B---C---D---E
>
> The sender's tip is at E and the receiver claims to have C. In
> the sender's repository, E is associated with A (somebody
> noticed that E fixes regression introduced by A, and added a
> softlink to make A reachable from E). Currently we only need to
> know C is reachable from E to decide that we do not have to send
> A again, but with softlink we would need to.
Hmm. First of all, I'm not sure it would be useful to add a _direct_ link
between E and A, but even so...
I'm thinking we can do the regular/current reachability calculation first,
and after it's done, we analyze the softrefs to see if there are more
objects to be fetched. In that scenario, we wouldn't need to send A again,
since it's already in our repo.
> The ancestry chain of referent and referer do not have to share
> any common commits. Imagine a bug tracking system where each
> bug's tracking sheet is represented as a DAG of commits (this
> will allow you to merge and split bugs easily). This history
> would not share any tree nor blob with the history of the source
> code of the project. And you would make a commit in the main
> project associated with objects in the bug tracking project
> using softrefs. As sender and receiver exchanges the commit
> ancestry information on the main project, both ends may need to
> negotiate which objects in the bug tracking history are already
> present in the reciever.
As above, if the receving side gets the list of involved softrefs, it can
make the decision on which objects it needs to fetch.
Hmm. Thinking about it, this process would of course need to be recursive,
which could potentially adversely affect the runtime of fetch...
> One attractive point of softref is that you do not have to
> anchor referents with explicit refs. E.g. if a commit in the
> main project is associated with bug tracking entries in the "bug
> tracking" project via softrefs, that is enough to keep the bug
> tracking objects alive. But I suspect this property makes the
> enumeration of "what do we have on this end" costly. I dunno.
Yeah, there are still may open questions. But I'm glad to see that most
people seem to find the basic concept useful, at least.
Thanks for taking the time and effort to comment.
Have fun!
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH] Fail if tag name and keywords is not within "printable ASCII"
From: Junio C Hamano @ 2007-06-10 1:33 UTC (permalink / raw)
To: Johan Herland; +Cc: Alex Riesen, git, Johannes Schindelin
In-Reply-To: <200706100235.24358.johan@herland.net>
Johan Herland <johan@herland.net> writes:
> Signed-off-by: Johan Herland <johan@herland.net>
> ---
>
> On Sunday 10 June 2007, Junio C Hamano wrote:
>> "Alex Riesen" <raa.lkml@gmail.com> writes:
>> > And what is so special about 0x7f?
>>
>> It is DEL, but as the code uses uchar, it probably also error on
>> 0x80 or higher, if the intent is "printable ASCII".
>
> Is this better?
I said "*if* the intent is to limit to printable ASCII".
It is still unclear what use cases the "keywords" need to
support (e.g. do we want to have "pick tags that have keyword
that matches this pattern"? if so, what kind of pattern
language do we want to use? Glob? Regexp? Would it be more
convenient if the keywords are treated case insensitively? Is
there a useful use case if you are allowed to use people's names
as keywords? Is it reasonable to assume that the keywords can
be split with a comma? Do we want to allow a comma as part of
keywords? If so, how would we quote a comma that is inside a
keyword? etc.). It depends on the answers to these questions
if "printable ASCII" is a good set.
As a general rule, if you make the initially allowed set of
values too wide, it gets _extremely_ hard to tighten it later.
So if we are to stick to printable ASCII (iow, "no people's
names or names of location as keywords"), I would even suggest
to limit the valid set further, say "^[-A-Za-z0-9_+.]+$", at
least initially.
^ permalink raw reply
* Re: git-svn set-tree bug
From: Eric Wong @ 2007-06-10 1:47 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: git
In-Reply-To: <1181323515.30670.110.camel@gentoo-jocke.transmode.se>
Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:
> trying to do git-svn set-tree remotes/trunk..svn
> in my new git-svn repo I get:
> config --get svn-remote.svn.fetch :refs/remotes/git-svn$: command returned error: 1
You need to specify "-i trunk" in the command-line
git-svn set-tree -i trunk remotes/trunk..svn
--
Eric Wong
^ permalink raw reply
* Re: Attributes for commits
From: Paul Franz @ 2007-06-10 2:10 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <200706092048.52142.robin.rosenberg.lists@dewire.com>
But the question is how many allow you to easily query this info. That
is one of the things I like about ClearCase's use of attributes is the
ability to query the repository about where it is set. For example, in
the case of git an attribute called "Status" on the commit could help
communicate to others what the status of a commit or better yet an
attribute on a branch. The one thing that I have realized is that change
management is all about communication and the repositories are just one
more way of communicating especially if the proper information can be
extracted and maintained by the repository. A good example of this is
CVS and merges. It is not CVS does merging poorly. It is that:
1) It has no mechanism in place for itself to know that the merge
has been taken place between different branches.
2) There is no way for people to know that the changes from one
branch has been merged to another.
The second is where meta data helps. In the case of ClearCase, they
something called a hyper-link that tells that a change to a file has
been merged from one branch to another and ClearCase uses this to know
how to merge future changes.
Now, I am not trying to turn git into ClearCase. I just see attributes
as a great way to help communicate things, from status of a branch to
what bug this change is related. Of course, part of adding attributes
would be adding a way to query git to find the commits or branches that
have the associated attributes.
Paul Franz
Robin Rosenberg wrote:
> lördag 09 juni 2007 skrev Paul Franz:
>
>> I am a ClearCase administrator and one of the things that I love
>> about it is the ability to assign attributes to versions of files. Is
>> there anybody thinking of adding the ability to assign attributes to a
>> commit? I ask because I was thinking about how I would move from
>> ClearCase to git and part of our process is to assign the BugNum
>> attribute to every checkin so that we can see what bugs have been fixed.
>> We also track the files checked in the bug tracking software to. Thus
>> giving us a two way linkage between repository and the bug tracking
>> system. This is VERY useful. And I was wondering if there are any
>> thought to this for commits.
>>
>> Now, I will be honest it is possible that this has already been done (I
>> have not read all the documentation yet) and I am justing wasting
>> bandwidth. In which, please excuse my ignorance and tell me to just RTFM.
>>
>> Paul Franz
>>
>>
>
> We use the convention of putting the bug number as the first work on the first line of
> the checkin, e.g. "3399 Add attributes for commits". The advantage over other methods
> is that it is always there, without the need to further investigations of assigned attributes.
>
> This convention has nothing to do with Git though. It works with any type of repo.
>
> -- robin
>
> -
> 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
>
>
--
-------------------------------------------
There are seven sins in the world.
Wealth without work.
Pleasure without conscience.
Knowledge without character.
Commerce without morality.
Science without humanity.
Worship without sacrifice.
Politics without principle.
-- Mohandas Gandhi
-------------------------------------------
^ permalink raw reply
* Re: [RFC] git integrated bugtracking
From: Daniel Barkalow @ 2007-06-10 2:44 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <20070609121244.GA2951@artemis>
On Sat, 9 Jun 2007, Pierre Habouzit 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/
I've been working on a completely orthogonal portion of the bug-tracking
problem (software to try to generate bug reports containing the
information that a particular project would like for a particular sort of
bug), and I've been thinking about how it would fit in with
git-the-content-addressed-filesystem. I haven't actually implemented
anything at all in this area, some you're ahead of me, but I have some
ideas.
1) It's probably best to use some new types, rather than trees and
commits. This gives you more flexibility to structure things in ways that
exactly fit what's going on, which is one of the main reasons git is so
good for version control. It also means that you can use inline strings
for short answers (what's the name of the program that makes your kernel
oops), and blobs for long answers (what's your lspci -vvv), and commits
when appropriate (what commit did git-bisect find? what version resolves
it?)
2) It's probably best to have the history be per-bug, with each revision
being an update to that report, and have the complete database be a refs/
subdirectory. The complete list of bugs is going to become vast, and it's
probably best to retain old bugs, at least in the databases at archival
sites, so that you can get information on how often a bug turns out to be
misuse of a particular API or something. This means that you really don't
want each addition to be O(number of bugs).
3) I think it's worth separately representing "what problem somebody had"
and "what was wrong with the program to cause problems" and linking these
to each other. This will help in being able to at least represent multiple
reports of the same bug, which is useful for finding patterns when the bug
is non-trivial.
My idea of what the structure of the data is:
- The project has essentially a troubleshooting procedure, written up
like a classic expert system (or like Kconfig). This takes the user
through a set of all the questions developers ever ask, with only the
appropriate ones visible (if you've got a build failure, it doesn't ask
for lspci output; it only asks for sysrq-t output if the system is
still sort of responding; etc).
- The failure report is the set of all the questions the user answered
and the answers.
- The hash of the initial failure report is the ID for the failure.
Revisions of the report (adding more information as people ask for
special things) retain the same ID.
- After you generate a failure report, it searches for similar failures
and bugs in the main database. If it finds stuff, the user can try any
resolutions, and skip sending the report in. If there's nothing there
or there's still uncertainty as to what to do about the issue or the
resolution doesn't work for this case, the report is added to the
database.
- It's up to developers to create bug records to pull together failure
reports, analysis of the situations, advice, and the ultimate
resolution. Failures get revised to link them to bugs so that people
with problems can find answers, and bugs list the failures they cause,
so that people working on something can find people to test.
- Reports can be made on (1) failures that somebody would be able to test
resolutions to, but which aren't attached to any bugs; (2) bugs that
aren't resolved in a particular commit (which may be resolved in some
later or parallel commit, or have a patch). These are the things that a
release engineer would want to check on before releasing.
- Reports can be made on clusters of unattached failures with similar
features. This would include unreproduceable failures, because they
might become fixable based on a large number of reports, or a test case
could be generated that makes them easier to trigger based on
distilling the common features of the rare failures.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: Comment on weak refs
From: Johannes Schindelin @ 2007-06-10 6:33 UTC (permalink / raw)
To: Johan Herland; +Cc: Junio C Hamano, git, Linus Torvalds, Pierre Habouzit
In-Reply-To: <200706100325.32846.johan@herland.net>
Hi,
On Sun, 10 Jun 2007, Johan Herland wrote:
> On Sunday 10 June 2007, Junio C Hamano wrote:
>
> > Maybe you first think A and B are related and then later change your
> > mind. Should "softref" relationships be versioned?
>
> Intriguing idea. Not immediately sure how we would implement it
> though...
Has my lightweight annotation patch reached you?
I like my approach better than yours, because it is
1) a way, way smaller patch, and
2) it automatically includes the versionability.
After thinking about it a little more (my plane was slow, and as a result
I am allowed to spend 8 more hours in Paris), I think that a small but
crucial change would make this thing even more useful:
Instead of having "core.showAnnotations" be a boolean config, it might be
better to have "core.annotationsRef" instead, overrideable by the
environment variable GIT_ANNOTATION_REF.
With this, you can have different refs for different kinds of annotations.
For example, some people might add bugtracker comments (even comments like
"this commit was bad: introduced bug #798, solved by commit 9899fdadc..").
Those comments could live in refs/annotations/bugs. To see them, just say
GIT_ANNOTATION_REF=refs/annotations/bugs gitk
Voila.
I am quite certain that treating annotations as branches, containing
fan-out directories for the reverse lookup. I am even quite certain that
in most cases, a working-directory-less merging is possible for such
annotations.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Silence error messages unless 'thorough_verify' is set
From: Johannes Schindelin @ 2007-06-10 6:48 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano
In-Reply-To: <200706092142.05446.johan@herland.net>
Hi,
On Sat, 9 Jun 2007, Johan Herland wrote:
> +#define FAIL(...) ( thorough_verify ? error(__VA_ARGS__) : -1 )
> +
> unsigned char sha1[20];
> char type[20];
> const char *type_line, *tag_line, *keywords_line, *tagger_line;
> @@ -80,26 +82,26 @@ int parse_and_verify_tag_buffer(struct tag *item,
> }
>
> if (size < 65)
> - return error("Tag object failed preliminary size check");
> + return FAIL("Tag object failed preliminary size check");
This is ugly.
If you _have_ to output the error message in one case, and not in the
other, I'd rather do
enum tag_error { TAG_SIZE_CHECK, TAG_BLA_BLUB, ... };
const char **tag_error_strings = { "tag: size error", ... };
Of course, you'd lose the ability to output some numbers. But those
numbers that you output are even uglier than the code. Guess how surprised
_I_ was, when I hit the error message which made me go mad.
Having said that, I still do not agree in this unifying.
Your rationale seems to be: use the same checking for the tag creation as
for the tag validation.
But this is _wrong_. We _do_ have tags that do not conform to the strict
standards of git-tag, and even if we did _not_, it would _still_ be wrong
to be that strict when _reading_ tags.
To drive that point home: strict checking when creating tags is good.
Strict checking when reading tags is bad.
I strongly encourage keeping both validations separate.
You'd also avoid having that many lines which are well over the encouraged
80 character limit.
Ciao,
Dscho
^ permalink raw reply
* [PATCH 6/5] git-push: Update description of refspecs and add examples
From: Junio C Hamano @ 2007-06-10 6:59 UTC (permalink / raw)
To: git
In-Reply-To: <11813808973041-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This is primarily to describe the semantics [Patch 5/5] fixes.
Documentation/git-push.txt | 25 ++++++++++++++++++++++---
1 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 366c5db..665f6dc 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -53,9 +53,8 @@ side are updated.
+
`tag <tag>` means the same as `refs/tags/<tag>:refs/tags/<tag>`.
+
-A parameter <ref> without a colon is equivalent to
-<ref>`:`<ref>, hence updates <ref> in the destination from <ref>
-in the source.
+A parameter <ref> without a colon pushes the <ref> from the source
+repository to the destination repository under the same name.
+
Pushing an empty <src> allows you to delete the <dst> ref from
the remote repository.
@@ -98,6 +97,26 @@ the remote repository.
include::urls.txt[]
+
+Examples
+--------
+
+git push origin master::
+ Find a ref that matches `master` in the source repository
+ (most likely, it would find `refs/heads/master`), and update
+ the same ref (e.g. `refs/heads/master`) in `origin` repository
+ with it.
+
+git push origin :experimental::
+ Find a ref that matches `experimental` in the `origin` repository
+ (e.g. `refs/heads/experimental`), and delete it.
+
+git push origin master:satellite/master::
+ Find a ref that matches `master` in the source repository
+ (most likely, it would find `refs/heads/master`), and update
+ the ref that matches `satellite/master` (most likely, it would
+ be `refs/remotes/satellite/master`) in `origin` repository with it.
+
Author
------
Written by Junio C Hamano <junkio@cox.net>, later rewritten in C
--
1.5.2.1.938.gac3b4
^ permalink raw reply related
* Re: [RFC] git integrated bugtracking
From: Martin Langhoff @ 2007-06-10 6:59 UTC (permalink / raw)
To: git
In-Reply-To: <20070609121244.GA2951@artemis>
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.
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.
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
If the bugtracker can also auto-link things that look committish in
text entered by users (someone might write "bisect sez that f345e is
to blame"), with tooltips indicating in which heads those commits
resides (like gitk does), then it's just gorgeous.
But I would _never_ try to describe all the possible relations in the
schema -- existing trackers use a liberal mix of regexes and cache
tables with some free form text fields for this kind of stuff.
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.
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%).
cheers,
m
^ permalink raw reply
* Re: [PATCH 1/7] Softrefs: Add softrefs header file with API documentation
From: Johannes Schindelin @ 2007-06-10 6:58 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano, Linus Torvalds
In-Reply-To: <200706092021.43314.johan@herland.net>
Hi,
On Sat, 9 Jun 2007, Johan Herland wrote:
> See patch for documentation.
This is preposterous. Either you substitute the patch for a documentation,
or you document it in the commit message. I consider commit messages like
"See patch for documentation" as reasonable as all those CVS "** no
message **" abominations.
> + * The softrefs db consists of two files: .git/softrefs.unsorted and
> + * .git/softrefs.sorted. Both files use the same format; one softref per line
> + * of the form "<from-sha1> <to-sha1>\n". Each sha1 sum is 40 bytes long; this
> + * makes each entry exactly 82 bytes long (including the space between the sha1 + * sums and the terminating linefeed).
> + *
> + * The entries in .git/softrefs.sorted are sorted on <from-sha1>, in order to
> + * make lookup fast.
> + *
> + * The entries in .git/softrefs.unsorted are _not_ sorted. This is to make
> + * insertion fast.
This sure sounds like you buy the disadvantages of both. Last time I
checked, it was recommended to look at your needs and pick _one_
appropriate data structure fitting _all_ your needs.
Besides, your lines are way too long. Yes, it is not in
Documentation/SubmittingPatches, but even just a cursory look into the
existing source shows you that it is mostly 80-chars-per-line. I think it
goes without saying that you should try to imitate the existing practices
in any project, and since you have to read the source to get acquainted
with it _anyway_, it would only be a duplication to have it in
SubmittingPatches, too.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/2] filter-branch: Simplify parent computation.
From: Johannes Schindelin @ 2007-06-10 7:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, git
In-Reply-To: <7vmyz8eu3y.fsf@assigned-by-dhcp.cox.net>
Hi,
On Sat, 9 Jun 2007, Junio C Hamano wrote:
> Anyway, I've manually fixed up the offending three patches (two from you
> and one from the other Johannes) and pushed the results out on 'next'.
Sorry. I guess we were just beaming patches back and forth, applying them
in our local repos, and working from there.
I promise to try better.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/3] git-submodule: allow submodule name and path to differ
From: Johannes Schindelin @ 2007-06-10 7:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Lars Hjemli, Sven Verdoolaege, git
In-Reply-To: <7vbqfod57b.fsf@assigned-by-dhcp.cox.net>
Hi,
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...
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC] git integrated bugtracking
From: Junio C Hamano @ 2007-06-10 7:35 UTC (permalink / raw)
To: Martin Langhoff; +Cc: git
In-Reply-To: <46a038f90706092359i43a6e834rc096e53a28fbee51@mail.gmail.com>
"Martin Langhoff" <martin.langhoff@gmail.com> writes:
> 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.
>
> 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.
After looking at the above existing alternatives, some brave
soul might decide and say, "Hey, I can write something better in
2 weeks" ;-).
> 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.
> If the bugtracker can also auto-link things that look committish in
> text entered by users (someone might write "bisect sez that f345e is
> to blame"), with tooltips indicating in which heads those commits
> resides (like gitk does), then it's just gorgeous.
>
> But I would _never_ try to describe all the possible relations in the
> schema -- existing trackers use a liberal mix of regexes and cache
> tables with some free form text fields for this kind of stuff.
These are indeed very good points.
^ permalink raw reply
* Re: [PATCH] Add --no-reuse-delta option to git-gc
From: Sam Vilain @ 2007-06-10 7:40 UTC (permalink / raw)
To: Steven Grimm
Cc: Shawn O. Pearce, Junio C Hamano, Daniel Barkalow,
Theodore Ts'o, Git Mailing List
In-Reply-To: <20070509191052.GD3141@spearce.org>
Shawn O. Pearce wrote:
>> On that note, has any thought been given to looking at other compression
>> algorithms? Gzip is a great high-speed compressor, but there are others
>> out there (some a bit slower, some much slower at both compression and
>> decompression) that produce substantially smaller output.
>>
> Its been discussed once before on the list, in very recent history,
> but not by a whole lot. As Junio pointed out, I don't think there
> ever really was any discussion of is gzip the best way to deflate the
> objects. I think gzip was just chosen simply because it was readily
> available in libz, stable, and has a pretty decent speed/size ratio.
>
I think it's the right tool. I just don't see any point in changing to
anything slower for the sake of 20% space saving. Especially bzip2.
Consider this.
Compression works primarily through two things: huffman coding and
string matching. The larger the window for your string matching, the
slower the compression and the more memory you need thrashing your CPU
memory cache when decompressing.
Now I'm not an expert on compression algorithms but I think a large part
of the reason gzip is blindingly faster than bzip2 is because gzip uses
a 64k buffer and bzip2 a 900k one. Only now are CPUs getting caches
large enough to deal with that size of buffer, the rest of the time
you're waiting for your RAM. Moore's law was supposed to make bzip2 fast
one of these days but I'm still waiting.
But with git-repack the window is effectively the size of your
repository. So that blows bzip2 out of the water. Why else can git make
compressed packs smaller than a .bz2 of the raw files? This is the same
observation Shawn makes with the pack-wide dictionary, but he sounds
like he wants to apply it to the huffman coding stage as well as the
current delta/string matching stage. Now that would be interesting...
Anyway it's a free world so be my guest to implement it, I guess if this
was selectable it would only be a minor annoyance waiting a bit longer
pulling from from some repositories, and it would be interesting to see
if it did make a big difference with pack file sizes.
Sam
^ permalink raw reply
* Re: [PATCH 1/7] Softrefs: Add softrefs header file with API documentation
From: Junio C Hamano @ 2007-06-10 7:43 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Johan Herland, git, Linus Torvalds
In-Reply-To: <Pine.LNX.4.64.0706100750440.4059@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Besides, your lines are way too long. Yes, it is not in
> Documentation/SubmittingPatches,...
> ... since you have to read the source to get acquainted
> with it _anyway_, it would only be a duplication to have it in
> SubmittingPatches, too.
Well, maybe we should do this.
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 01354c2..4bdfdfe 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -5,6 +5,7 @@ Checklist (and a short version for the impatient):
- make commits of logical units
- check for unnecessary whitespace with "git diff --check"
before committing
+ - tab width is 8, the terminal is 80-columns wide.
- do not check in commented out code or unneeded files
- provide a meaningful commit message
- the first line of the commit message should be a short
@@ -82,6 +83,14 @@ option).
Another thing: NULL pointers shall be written as NULL, not as 0.
+(1b) Tab width is 8, Terminal is 80-column wide.
+
+We generally follow the same coding style guidelines as the
+Linux kernel project. Lines are indented with Tabs, each of
+which are 8 columns wide. Lines should fit on 80-column wide
+terminals.
+
+
(2) Generate your patch using git tools out of your commits.
git based diff tools (git, Cogito, and StGIT included) generate
^ permalink raw reply related
* Re: Is this an acceptable workflow in git-svn, or a user error?
From: Sam Vilain @ 2007-06-10 7:44 UTC (permalink / raw)
To: Eric Wong; +Cc: Junio C Hamano, git
In-Reply-To: <20070609193835.GB32225@muzzle>
Eric Wong wrote:
>> Is my understanding correct?
>>
>
> Yes. Since dcommit uses rebase, it'll rewrite history.
>
Maybe these new lightweight annotations could be a way around that?
Sam.
^ permalink raw reply
* Re: [RFC] git integrated bugtracking
From: Johannes Schindelin @ 2007-06-10 7:44 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Pierre Habouzit, git
In-Reply-To: <Pine.LNX.4.64.0706092152180.5848@iabervon.org>
Hi,
On Sat, 9 Jun 2007, Daniel Barkalow wrote:
> 1) It's probably best to use some new types, rather than trees and
> commits. This gives you more flexibility to structure things in ways
> that exactly fit what's going on, which is one of the main reasons git
> is so good for version control.
I fail to see why this has to be a new type. The flexibility in Git lies
IMHO therein that it does _not_ have a plethora of objects. Rather, there
are just 4 object types, which serve their purpose well, indeed. (I would
even have argued that tag objects could have been simple commit objects,
with an additional header "tag", but oh well.)
I suspect that you want to introduce a different object type to be able to
implement a new algorithm. But I think that the appropriate data structure
is still contained in the existing set of types in Git.
> 2) It's probably best to have the history be per-bug, with each revision
> being an update to that report, and have the complete database be a
> refs/ subdirectory.
I don't think that this is a good solution:
>From the implementation view point, a lot of branches sucks
performance-wise. Especially since we do not pack branch refs.
Side note: I recently kicked around the idea to actually keep
the refs in the packed-refs file, and for updating refs do a
lock-mmap-findref-replace-or-rewrite, where a rewrite only happens if we
delete or insert a ref.
Then, we could even unify the info/refs and packed-refs, so that we don't
hear bugreports about http transport not working every week or so.
Just an idea.
Ciao,
Dscho
^ permalink raw reply
* Re: Is this an acceptable workflow in git-svn, or a user error?
From: Sam Vilain @ 2007-06-10 7:49 UTC (permalink / raw)
To: Eric Wong; +Cc: Junio C Hamano, git
In-Reply-To: <466BABFB.8080208@vilain.net>
Sam Vilain wrote:
> Eric Wong wrote:
>
>>> Is my understanding correct?
>>>
>> Yes. Since dcommit uses rebase, it'll rewrite history.
>>
> Maybe these new lightweight annotations could be a way around that?
>
Actually I don't think that will work either unless you start
encapsulating extra git information (author/committer email/timestamps)
in the svn commit somewhere, so that someone else fetching the commits
from svn directly will end up with the same git commitid. And then
there's still the issue of merges.
Sam.
^ permalink raw reply
* Re: [PATCH 04/21] Refactor verification of "tagger" line to be more similar to verification of "type" and "tagger" lines
From: Johannes Schindelin @ 2007-06-10 7:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johan Herland, git
In-Reply-To: <7vr6olf1m7.fsf@assigned-by-dhcp.cox.net>
Hi,
On Sat, 9 Jun 2007, Junio C Hamano wrote:
> Johan Herland <johan@herland.net> writes:
>
> > + if (!tag_line++)
> > return error("char" PD_FMT ": could not find next \"\\n\"", type_line - data);
> > - tag_line++;
BTW if you _are_ verbosing the output, you might just as well make it
useful.
The common format is "filename:line[:column]:message", not "char[n]",
which is a misnomer to begin with, since you are talking about an offset,
not a char (remember, characters are those things that are displayed in
place for a given number, so I fully expected char32 to be a space).
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/7] Softrefs: Add softrefs header file with API documentation
From: Johannes Schindelin @ 2007-06-10 7:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johan Herland, git, Linus Torvalds
In-Reply-To: <7vzm389rvl.fsf@assigned-by-dhcp.cox.net>
Hi,
On Sun, 10 Jun 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Besides, your lines are way too long. Yes, it is not in
> > Documentation/SubmittingPatches,...
> > ... since you have to read the source to get acquainted
> > with it _anyway_, it would only be a duplication to have it in
> > SubmittingPatches, too.
>
> Well, maybe we should do this.
>
> [...]
But where to stop?
Many people want to put an opening curly bracket in its own line. Other
indenting is subject for discussion, too. White space after operators, but
not after function names should be included, too.
I know you mean good, but I think it is not a bad idea to let people get
familiar with the code (and the formatting rules) first. This way we can
even tell who did, and who did not do that, before submitting a patch.
Ciao,
Dscho
^ 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 8:02 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano, Linus Torvalds
In-Reply-To: <200706092025.30156.johan@herland.net>
Hi,
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.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 05/21] Make parse_tag_buffer_internal() handle item == NULL
From: Johannes Schindelin @ 2007-06-10 8:06 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano
In-Reply-To: <200706090214.39337.johan@herland.net>
Hi,
On Sat, 9 Jun 2007, Johan Herland wrote:
> - tag_len = tagger_line - tag_line - strlen("tag \n");
> - item->tag = xmalloc(tag_len + 1);
> - memcpy(item->tag, tag_line + 4, tag_len);
> - item->tag[tag_len] = '\0';
> -
> - if (!strcmp(type, blob_type)) {
> - item->tagged = &lookup_blob(sha1)->object;
> - } else if (!strcmp(type, tree_type)) {
> - item->tagged = &lookup_tree(sha1)->object;
> - } else if (!strcmp(type, commit_type)) {
> - item->tagged = &lookup_commit(sha1)->object;
> - } else if (!strcmp(type, tag_type)) {
> - item->tagged = &lookup_tag(sha1)->object;
> - } else {
> - error("Unknown type %s", type);
> - item->tagged = NULL;
> - }
> -
> - if (item->tagged && track_object_refs) {
> - struct object_refs *refs = alloc_object_refs(1);
> - refs->ref[0] = item->tagged;
> - set_object_refs(&item->object, refs);
> + if (item) {
> + tag_len = tagger_line - tag_line - strlen("tag \n");
> + item->tag = xmalloc(tag_len + 1);
> + memcpy(item->tag, tag_line + 4, tag_len);
> + item->tag[tag_len] = '\0';
> +
> + if (!strcmp(type, blob_type)) {
> + item->tagged = &lookup_blob(sha1)->object;
> + } else if (!strcmp(type, tree_type)) {
> + item->tagged = &lookup_tree(sha1)->object;
> + } else if (!strcmp(type, commit_type)) {
> + item->tagged = &lookup_commit(sha1)->object;
> + } else if (!strcmp(type, tag_type)) {
> + item->tagged = &lookup_tag(sha1)->object;
> + } else {
> + error("Unknown type %s", type);
> + item->tagged = NULL;
> + }
> +
> + if (item->tagged && track_object_refs) {
> + struct object_refs *refs = alloc_object_refs(1);
> + refs->ref[0] = item->tagged;
> + set_object_refs(&item->object, refs);
> + }
> }
>
> return 0;
The patch would have been so much simpler, and so much more descriptive of
what you're doing, had you just said
if (!item)
return 0;
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Silence error messages unless 'thorough_verify' is set
From: Junio C Hamano @ 2007-06-10 8:15 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Johan Herland, git
In-Reply-To: <Pine.LNX.4.64.0706100741310.4059@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Sat, 9 Jun 2007, Johan Herland wrote:
> ...
>> @@ -80,26 +82,26 @@ int parse_and_verify_tag_buffer(struct tag *item,
>> }
>>
>> if (size < 65)
>> - return error("Tag object failed preliminary size check");
>> + return FAIL("Tag object failed preliminary size check");
>
> This is ugly.
... quite a bit. A less uglier alternative we seem to use in
other places is not much better (return NULL on failure or an
error message string on error).
> ... Guess how surprised
> _I_ was, when I hit the error message which made me go mad.
To be fair, that ugly "char%d" was taken from mktag and not
Johan's invention.
> To drive that point home: strict checking when creating tags is good.
> Strict checking when reading tags is bad.
>
> I strongly encourage keeping both validations separate.
While I tend ot think that keeping two separate versions is
probably better for this particular case, the above statement
has a leap in its logic. With your "error code" scheme, you
could implement a single, verifier/parser that defines the
concrete and complete rule of how the data should look like.
That unified verifier/parser itself should be silent. Then, you
can have each of the callers decide how lenient it wants to be,
depending on the seriousness of the error. You can make
producer very strict and chatty while leaving consumer liberal
and more silent.
There are pros-and-cons, however.
- Such a scheme to return error codes and have two callers that
have different behaviours is cumbersome to set up and use.
A good example of this is the switch/case mess in each of the
callers of run_command_v_opt() in builtin-push.c,
builtin-revert.c, receive-pack.c etc. For run_command, the
mess is justifiable because the function has enough number of
different callers, but in the current thread, we are only
talking about two callers (parsing vs verifying of tag
objects).
- It has a risk to introduce inconsitent definition of the data
format to have completely separate producer and consumer
implementations; this is especially true when the data in
question is complex.
However, a tag is sufficiently simple that my personal
feeling is that, combined with the cumbersomeness argument
against the unified verifier, separate producer and consumer
implementations would be easier to manage for this particular
case.
^ 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 8:14 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Alex Riesen, Junio C Hamano
In-Reply-To: <200706092334.32855.johan@herland.net>
Hi,
On Sat, 9 Jun 2007, Johan Herland wrote:
> On Saturday 09 June 2007, Alex Riesen wrote:
> > On 6/9/07, Johan Herland <johan@herland.net> wrote:
> > > Signed-off-by: Johan Herland <johan@herland.net>
> > > ---
> > > mktag.c | 29 ++++++++++++++++-------------
> > > 1 files changed, 16 insertions(+), 13 deletions(-)
> >
> > What is this change good for?
> > How did you justify the type selection for your
> > loop index variable?
> >
> > IOW, the patch looks very useless.
>
> I agree. By itself, the patch is useless.
Then it shouldn't be there.
It seems that you do not place the cuts between patches at the
_conceptual_ layer. Therefore, they seem intrusive and often the meaning
evades me.
So, if I understood the purpose of this patch series correctly, namely to
use the same verification routines both for creation as for validation of
tags, you could have
- moved one function into the library (the stricter one), saying
"move this_function() into libgit.a to make it usable from
git-bla" in the commit body,
- used that from the other program, removing the now-unused
function,
- and then changed the behaviour to be more chatty or some such.
As it is, you have a mix of conceptually different changes in almost every
patch, and some changes that conceptually belong into the same patch, are
not.
Be that as may, I think it is not a good change to reuse the same function
like you did, exactly because one version _should_ be more forgiving than
the other.
Ciao,
Dscho
^ permalink raw reply
* 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
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