[PATCH 0/2] tagsize < 8kb restriction

[PATCH 0/2] tagsize < 8kb restriction

[Björn Engelmann]

Hi,

I am currently working on an interface for source code quality assurance
tools to automatically scan newly commited code. Since it is the only
way to add data (scan-results) to an already-existing commit, I decided
to use tags for that.

Since the scan-results will most definitly exeed the 8kb-limit, I would
like to remove this artificial restriction.

I know I could also

    git-hash-object -t tag -w

but prefer using the "official" way.


In order not to write duplicate code I used parts of index_pipe() in
sha1_file.c

What I found odd when writing the patch was that main() in mktag.c uses
xread() to read from stdin (which respects EAGAIN and EINTR return
values), but index_pipe() in sha1_file.c just uses read() for doing
merely the same thing. For unifying both routines i found that xread()
might be the better choice.

Removing the restriction was pretty straightforward but do you think
this would break something in other places ?


[PATCH 1/2]
    remove the < 8kb restrinction from git-mktag

[PATCH 2/2]
    add more informative error messages to git-mktag


Bj

Re: [PATCH 0/2] tagsize < 8kb restriction

[Junio C Hamano]

Björn Engelmann <BjEngelmann@gmx.de> writes:

> I am currently working on an interface for source code quality assurance
> tools to automatically scan newly commited code. Since it is the only
> way to add data (scan-results) to an already-existing commit, I decided
> to use tags for that.
>
> Since the scan-results will most definitly exeed the 8kb-limit, I would
> like to remove this artificial restriction.

Lifting the limit is good, but I am not sure if the use of tags
for that purpose is appropriate (or any git object for that
matter).  I'll talk about that at the end.

> What I found odd when writing the patch was that main() in mktag.c uses
> xread() to read from stdin (which respects EAGAIN and EINTR return
> values), but index_pipe() in sha1_file.c just uses read() for doing
> merely the same thing. For unifying both routines i found that xread()
> might be the better choice.

Good.

> Removing the restriction was pretty straightforward but do you think
> this would break something in other places ?

I do not think so offhand.

Now, about the usage of such a long tag for your purpose.

As you noticed, commits and tags are the only types of objetcs
that can refer to other commits structurally.  But there are
cases where you do not even need nor want structural reference.
For example, 'git cherry-pick' records the commit object name of
the cherry-picked commit in the commit message as part of the
text -- such a commit does not have structural reference to the
original commit, and we would not _want_ one.  I have a strong
suspicion that your application does not need or want structural
reference to commits, and it might be better to merely mention
their object names as part of the text the application produces,
just like what 'git cherry-pick' does.

Presumably you will have one such tag per commit, and by default
'fetch' (both cg and git) tries to follow tags, which means
anybody who fetches new revision would automatically download
this QA data -- that is one implication of using a tag to store
this information.  Without knowing the nature of it, I am not
sure if everybody who tracks the source wants such baggage.  If
not, then use of a tag for this may not be appropriate.

Another question is if the QA data expected to be amended or
annotated later, after it is created.

If the answer is yes, then you probably would not want tags --
you can create a new tag that points at the same commit to
update the data, but then you have no structural relationships
given by git between such tags that point at the same commit.
You could infer their order by timestamp but that is about it.
I think you are better off creating a separate QA project that
adds one new file per commit on the main project, and have the
file identify the commit object on the main project (either
start your text file format for QA data with the commit object
name, or name each such QA data file after the commit object
name).  Then your automated procedure could scan and add a new
file to the QA project every time a new commit is made to the
main project, and the data in the QA project can be amended or
annotated and the changes will be version controlled.

If the answer is no, then it is probably better to just use an
append-only log file that textually records which entry
corresponds to which commit in the project.  If it is not
version controlled, and if it is not part of the main project, I
do not see much point in putting the data under git control and
in the same project.

Re: [PATCH 0/2] tagsize < 8kb restriction

[Sean]

On Mon, 22 May 2006 12:18:04 -0700
Junio C Hamano <junkio@cox.net> wrote:

> Now, about the usage of such a long tag for your purpose.
> 
> As you noticed, commits and tags are the only types of objetcs
> that can refer to other commits structurally.  But there are
> cases where you do not even need nor want structural reference.
> For example, 'git cherry-pick' records the commit object name of
> the cherry-picked commit in the commit message as part of the
> text -- such a commit does not have structural reference to the
> original commit, and we would not _want_ one.  I have a strong
> suspicion that your application does not need or want structural
> reference to commits, and it might be better to merely mention
> their object names as part of the text the application produces,
> just like what 'git cherry-pick' does.

What seems to becoming clear as more people find new ways to use
git is that many of them would be well served by having a solid
infrastructure to handle metadata.  Consider the case above: _git_
itself doesn't need a structural reference, but users and external
applications definitely need to be able to lookup which metadata
is associated with any given commit.  Having a git standard for
this type of data would help.  Tags already do this, so they're
likely to be used and abused in ways not initially envisioned,
just because git doesn't have another such facility.
 
> Presumably you will have one such tag per commit, and by default
> 'fetch' (both cg and git) tries to follow tags, which means
> anybody who fetches new revision would automatically download
> this QA data -- that is one implication of using a tag to store
> this information.  Without knowing the nature of it, I am not
> sure if everybody who tracks the source wants such baggage.  If
> not, then use of a tag for this may not be appropriate.

Right.  It would be much nicer if it was possible to request or
ignore specific types of metadata when fetching; yet another
reason that it would be great if git had something built in
which anticipated this need.
 
> Another question is if the QA data expected to be amended or
> annotated later, after it is created.
> 
> If the answer is yes, then you probably would not want tags --
> you can create a new tag that points at the same commit to
> update the data, but then you have no structural relationships
> given by git between such tags that point at the same commit.
> You could infer their order by timestamp but that is about it.
> I think you are better off creating a separate QA project that
> adds one new file per commit on the main project, and have the
> file identify the commit object on the main project (either
> start your text file format for QA data with the commit object
> name, or name each such QA data file after the commit object
> name).  Then your automated procedure could scan and add a new
> file to the QA project every time a new commit is made to the
> main project, and the data in the QA project can be amended or
> annotated and the changes will be version controlled.

There are a lot of nice features with using a separate meta-data
branch.  However, you lose the ability to do lookups like you can
with tags.  A tag like index that gave the ability to associate
commits on otherwise unrelated branches might be a way to get
the best of both worlds.  However, there will be times where
version controlled meta-data is overkill.  Just need to codify a
git-standard for meta data, so that git can help where possible.
 
> If the answer is no, then it is probably better to just use an
> append-only log file that textually records which entry
> corresponds to which commit in the project.  If it is not
> version controlled, and if it is not part of the main project, I
> do not see much point in putting the data under git control and
> in the same project.

It would be very nice if git gave a standard way to lookup and
perhaps even display metadata.   Could add an option to git log
for example that said, show all metadata of a certain type.

There are a limitless number of examples where people want to
associate extra information with each commit.  Other SCM's call
these "attributes" or have other such names.  Given git's design
it isn't too hard to imagine offering the ability for version 
controlled (or not) and public (or not) meta-data.  Very similar
to tags, but perhaps with a few extra features.

If git already offered this feature, there'd be no need for a
flat-file ref-log; the data could be stored in a git-standard
way for metadata and gain the features of whatever tools grow
up around it, like querying, inspecting, purging etc..  All of
a sudden people would be able to look at (and perhaps even update)
their own meta data via git log/qgit/gitk/gitweb etc..   All we
need is a standard that everyone can conform with.

Sean

Re: [PATCH 0/2] tagsize < 8kb restriction

[Linus Torvalds]

On Mon, 22 May 2006, Sean wrote:
> What seems to becoming clear as more people find new ways to use
> git is that many of them would be well served by having a solid
> infrastructure to handle metadata.  Consider the case above: _git_
> itself doesn't need a structural reference, but users and external
> applications definitely need to be able to lookup which metadata
> is associated with any given commit.  Having a git standard for
> this type of data would help.  Tags already do this, so they're
> likely to be used and abused in ways not initially envisioned,
> just because git doesn't have another such facility.

I definitely think we should allow arbitrary tags.

That said, I think that what you actually want to do may be totally 
different.

If _each_ commit has some extra information associated with it, you don't 
want to create a tag that points to the commit, you more likely want to 
create an object that is indexed by the commit ID rather than the other 
way around.

IOW, I _think_ that what you described would be that if you have the 
commit ID, you want to find the data based on that ID. No?

And that you can do quite easily, while _also_ using git to distribute the 
extra per-commit meta-data. Just create a separate branch that has the 
data indexed by commit ID. That could be as simple as having one file per 
commit (using, perhaps, a similar directory layout as the .git/objects/ 
directory itself), and then you could do something like

	# Get the SHA1 of the named commit
	commit=$(git-rev-parse --verify "$cmitname"^0)

	# turn it into a filename (slash between two first chars and the rest)
	filename=$(echo $commit | sed 's:^\(..\)\(.*\):\1/\2:')

	# look it up in the "annotations" branch
	git cat-file blob "annotations:$filename"

which gets the data from the "annotations" branch, indexed by the SHA1 
name.

Now, everybody can track your "annotations" branch using git, and get your 
per-commit annotations for the main branch.

See?

The real advantage of tags is that you can use them for the SHA1 
expressions, and follow them automatically. If that's what you want (ie 
you don't want to index things by the commit SHA1, but by some external 
name, like the name the commit had in some other repository), then by all 
means use tags. But if you just want to associate some data with each 
commit, the above "separate branch for annotations" approach is much more 
efficient.

		Linus

Re: [PATCH 0/2] tagsize < 8kb restriction

[Sean]

On Mon, 22 May 2006 17:02:41 -0700 (PDT)
Linus Torvalds <torvalds@osdl.org> wrote:

> That said, I think that what you actually want to do may be totally 
> different.
> 
> If _each_ commit has some extra information associated with it, you don't 
> want to create a tag that points to the commit, you more likely want to 
> create an object that is indexed by the commit ID rather than the other 
> way around.
> 
> IOW, I _think_ that what you described would be that if you have the 
> commit ID, you want to find the data based on that ID. No?

Correct.  But even though there isn't currently an efficient way to do
the reverse lookup, many of the visual tools do it.  So for instance, if
you embed some metadata in the name of a tag, it will actually be nicely
shown to you associated with the proper commit in qgit/gitk/gitweb.  So
you _can_ abuse tags and get modest results.  And of course the low level
tools let you do..  git name-rev --tags to lookup the meta data as well.
So you can do  git log | git name-rev --tags --stdin  and see which tags
are associated with each commit.

I'm not arguing that this makes tags well designed for the these types
of things, just that there is no other option built in to the low level
git that comes as close.

> And that you can do quite easily, while _also_ using git to distribute the 
> extra per-commit meta-data. Just create a separate branch that has the 
> data indexed by commit ID. That could be as simple as having one file per 
> commit (using, perhaps, a similar directory layout as the .git/objects/ 
> directory itself), and then you could do something like
> 
> 	# Get the SHA1 of the named commit
> 	commit=$(git-rev-parse --verify "$cmitname"^0)
> 
> 	# turn it into a filename (slash between two first chars and the rest)
> 	filename=$(echo $commit | sed 's:^\(..\)\(.*\):\1/\2:')
> 
> 	# look it up in the "annotations" branch
> 	git cat-file blob "annotations:$filename"
> 
> which gets the data from the "annotations" branch, indexed by the SHA1 
> name.
> 
> Now, everybody can track your "annotations" branch using git, and get your 
> per-commit annotations for the main branch.
> 
> See?

Sure, makes a lot of sense.  Although the one file per commit thing doesn't
scale well.  It's already a problem when trying to use a lot of tags for
instance.

More than the technical details of the implementation though, i'm trying to
make a case that git would do well to codify something like the above and
provide a _standard_ method of associating meta data with commits.  This
would allow all the tools to start displaying meta data etc and have a
defined way to efficiently query and manipulate it. 

Sean

Re: [PATCH 0/2] tagsize < 8kb restriction

[Björn Engelmann]

Hi,

I hope this time I got it right. Is there some kind of style-guide I can
refer to in future ?


> Another question is if the QA data expected to be amended or
> annotated later, after it is created.
>
> If the answer is yes, then you probably would not want tags --
> you can create a new tag that points at the same commit to
> update the data, but then you have no structural relationships
> given by git between such tags that point at the same commit.
> You could infer their order by timestamp but that is about it.
> I think you are better off creating a separate QA project that
> adds one new file per commit on the main project, and have the
> file identify the commit object on the main project (either
> start your text file format for QA data with the commit object
> name, or name each such QA data file after the commit object
> name).  Then your automated procedure could scan and add a new
> file to the QA project every time a new commit is made to the
> main project, and the data in the QA project can be amended or
> annotated and the changes will be version controlled.
>   

Great idea ! Thanks a lot. Originally it was not planned to alter the
results once committet, but this way it would even be possible to rescan
a commit with a different tool and merge the results. Git would also be
able to use delta-encoding when packing what can be considered extremly
efficient since most probably most scan-results won't differ much.

I am currently wondering where to store the reference to such a
sub-repository. It certainly is a head, but I would like to avoid anyone
commiting code into this "branch". Maybe I will create a new directory
.git/refs/annotations.

When thinking about this very elegant way to handle meta-data, I got
another idea:
The quality assurance system also works distributed. For scalability
reasons there are multiple scanners, each scanning one commit at a time.
Do you think git could also be used to handle "locking" ? The scanners
would then push a commit with an empty result-file into the
annotations-repository so all other scanners who are looking for
currently unscanned commits would ignore it in future. When finished the
result can be inserted by pushing a subsequent commit. This way one
avoids the need for a seperate job-server / protocol.
I am not sure how git would perform in such an environment. Do you think
the "git-push"-implementation is sufficiently "thread-save" for this ?
Or could simultaniously pushing into the same branch f.e. break  the
repository ?

Hmm.. 2 more things on my mind:
1.) Do you intend to add some more advanced metadata-functionality to
git in the future or should I send a patch with my implementation once
it is finished ? Will be just some scripts using similar commands to
what Linus sent me (thanks for that, btw)

2.) Searching for a way to add objects to the database I spent quite a
while to find the right command. Don't you think it would be much more
intuitive having an

    git-create-object [-t <type>] [-n] [-f] [-z] [--stdin] <file> [-r
<ref-name>]

command for creating any type of object (-t blob as default), optionally
omitting writing it to the database (-n = no-write) (like
git-hash-object), by default validating its input  (overriding with -f)
(like git-mktag, git-mktree) and maybe even able to add a reference to
it with -r (like git-tag).

Bj

Re: [PATCH 0/2] tagsize < 8kb restriction

[Junio C Hamano]

Björn Engelmann <BjEngelmann@gmx.de> writes:

> I am currently wondering where to store the reference to such a
> sub-repository. It certainly is a head, but I would like to avoid anyone
> commiting code into this "branch". Maybe I will create a new directory
> .git/refs/annotations.

I would recommend against that.  Why shouldn't it be an ordinary
branch that is different from the default "master"?

If you are in a shared repository settings, then update hook is
there for you to prevent people who do not have any business
touching that branch head from mucking with it.

> I am not sure how git would perform in such an environment. Do you think
> the "git-push"-implementation is sufficiently "thread-save" for this ?

Yes.

And I do not necessarily think your workflow would want to have
such "an empty file works as a lock" convention.

Just do things locklessly.  If two people in the group happened
to do duplicated work, the first push would succeed and the
second person would be prevented from pushing (and suggested to
merge in the work first).  When the second person pulls, he
would realize the scan result by the first person is already
there.  If that is considered to be too much wasted work, then
it means your distributed workflow did not have sufficient
communication among people.  Being able to work distributed does
not mean you need no coordination, and a distributed SCM is not
a substitute for comminication among paticipants.

> 1.) Do you intend to add some more advanced metadata-functionality to
> git in the future or should I send a patch with my implementation once
> it is finished ? Will be just some scripts using similar commands to
> what Linus sent me (thanks for that, btw)

Neither, until/unless we have a clear design.

I think the annotation branch (or a separate repository) is a
very natural consequence of what the tool already give you, and
the tools work just fine as they are.  There is nothing
innovative in what I suggested above nor Linus outlined in the
other message.

If you are talking about an application that builds on top of
git to do issue management (or QA or whatever), that uses
metadata linked to the commits on the main development branch,
that would be a wonderful system, but that does not necessarily
have to come with git (it's just an application on top of git,
and the workflow of your organization may or may not match other
people's workflow).

> 2.) Searching for a way to add objects to the database I spent quite a
> while to find the right command. Don't you think it would be much more
> intuitive having an
>
>     git-create-object [-t <type>] [-n] [-f] [-z] [--stdin] <file> [-r
> <ref-name>]
>
> command for creating any type of object (-t blob as default).

No, I do not think we would want to make it too easy and relaxed
to create arbitrary object-looking thing.  Each type have
defined format and semantics, and creation of an object of each
type should be validated.  I do not want to encourage bypassing
it by introducing such a backdoor.  The backdoor is easy to
write, but I suspect it would actively harm us, instead of
helping us, by encouraging "let's build a custom type of object,
we do not care if other people would not understand it"
mentality.

Re: [PATCH 0/2] tagsize < 8kb restriction

[Junio C Hamano]

Björn Engelmann <BjEngelmann@gmx.de> writes:

> I hope this time I got it right.

Thanks.  Pushed out as a part of "next".  Will hopefully be part
of "master" by the end of the week if not earlier.

Re: [PATCH 0/2] tagsize < 8kb restriction

[Björn Engelmann]

>> 2.) Searching for a way to add objects to the database I spent quite a
>> while to find the right command. Don't you think it would be much more
>> intuitive having an
>>
>>     git-create-object [-t <type>] [-n] [-f] [-z] [--stdin] <file> [-r
>> <ref-name>]
>>
>> command for creating any type of object (-t blob as default).
>>     
>
> No, I do not think we would want to make it too easy and relaxed
> to create arbitrary object-looking thing.  Each type have
> defined format and semantics, and creation of an object of each
> type should be validated.  I do not want to encourage bypassing
> it by introducing such a backdoor.  The backdoor is easy to
> write, but I suspect it would actively harm us, instead of
> helping us, by encouraging "let's build a custom type of object,
> we do not care if other people would not understand it"
> mentality.
>   

Well, this is exactly what you have now in
    git-hash-object -w -t foo

That is why I said, all input should be validated by default. All I
proposed was
a) unify the tools in order to have less duplicate code (git-mktag,
git-mktree & git-hash-object do merely the same except for the
validating part)
b) remove the possibility to introduce unchecked objects of arbitrary
type (or only allow it with the -f = "force, use with caution"-option)

maybe I should have written "blob, tag, tree or commit" instead of
"arbitrary". I did not mean really arbitrary like it is implemented
right now in git-hash-object.

Bj

Re: [PATCH 0/2] tagsize < 8kb restriction

[Junio C Hamano]

Björn Engelmann <BjEngelmann@gmx.de> writes:

> That is why I said, all input should be validated by default. All I
> proposed was
> a) unify the tools in order to have less duplicate code
> (git-mktag, git-mktree & git-hash-object do merely the same
> except for the validating part)
> b) remove the possibility to introduce unchecked objects of arbitrary
> type (or only allow it with the -f = "force, use with caution"-option)
> maybe I should have written "blob, tag, tree or commit" instead of
> "arbitrary". I did not mean really arbitrary like it is implemented
> right now in git-hash-object.

Sorry, I forgot all about hash-objects X-<.  It was a convenient
way to try out new things such as 'gitlink'.  Thanks for the
clarification.

As to unification, I am not sure if there are a lot to unify.
Everybody starts with type, length and a LF, but after that each
type has its own format constraints.  A grand unified command
that knows about format constraints of every type under the sun
does not sound like a good approach.  While we have only handful
types (and I expect things will stay that way) it is not a big
deal either way, though.

And the common part is already shared (write_sha1_file_prepare()
and write_sha1_file() from sha1_file.c).

Re: [PATCH 0/2] tagsize < 8kb restriction

[Björn Engelmann]

> Sorry, I forgot all about hash-objects X-<.  It was a convenient
> way to try out new things such as 'gitlink'.  Thanks for the
> clarification.
>
> As to unification, I am not sure if there are a lot to unify.
> Everybody starts with type, length and a LF, but after that each
> type has its own format constraints.  A grand unified command
> that knows about format constraints of every type under the sun
> does not sound like a good approach.  While we have only handful
> types (and I expect things will stay that way) it is not a big
> deal either way, though.
>   
Oops, sorry, I forgot that "modular" in C means something else than in
the OO-World...
You are right. Probably it is best to have one tool handle each type.

Actually what I am aiming for is not the internal structure. I am more
concerned about cleaning up the user-interface. When I started learning
git I found it very annoying and inconsistent that there are commands
for creating a tag and a tree in a validated fashion, but the command
for creating blobs was named "git-hash-object -w" and also could create
all other objects without validating them at all. Also, AFAIK there is
currently no way of creating a commit object with validating.

I am well aware that all functionality neccessary already exists. I just
want to prevent people learning git in future to have the same
frustrating experience as I did.

Obviously renaming / moving code around like that would break nearly all
tools build ontop of git. Therefore I would prefer to use aliasing. If
you feel like this would introduce too many unneccessary commands, I
would instead focus on improving the documentation.

Bj

Re: [PATCH 0/2] tagsize < 8kb restriction

[Junio C Hamano]

Björn Engelmann <BjEngelmann@gmx.de> writes:

> I am well aware that all functionality neccessary already exists. I just
> want to prevent people learning git in future to have the same
> frustrating experience as I did.

I think I understood your points, but for normal "people
learning git", hash-object, write-tree, commit-tree and mktag
are _not_ the commands they need to know about.  These low level
commands are for Porcelain writers.  The users do not create
blobs or trees or commits -- they "git add", "git rm", "git
commit", and "git pull" and as part of these actions, blobs,
trees and commits are created.  The users do not even create
tags with mktag -- they use "git tag" for that.