Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lightweight tags #141

Closed
schwern opened this issue Sep 18, 2014 · 8 comments
Closed

Lightweight tags #141

schwern opened this issue Sep 18, 2014 · 8 comments

Comments

@schwern
Copy link
Contributor

schwern commented Sep 18, 2014

I would like to make use of Git::Raw in Gitpan, but it's missing support for lightweight tags. Gitpan needs to make a lot of tags and full, cryptographically secure, annotated tags are overkill. I realize they can be created with Git::Repository::Reference->create, but lightweight tags are not well integrated into the rest of Git::Raw.

I began to implement it, but ran into a problem. Git::Raw::Tag represents an annotated tag object. Lightweight tags do not appear to have a tag object, they're references to a commit.

Where I'm stuck in implementing lightweight tags is where they fit relative to annotated tags. Should they be left as references so Git::Raw::Reference->create_lightweight_tag() is a wrapper around Git::Raw::Reference->create("refs/tags/$name", $repo, $commit)?

Or should Git::Tag attempt to integrate them? I noticed there's a test in t/03-tag.t which appears to deliberately check that Git::Raw::Repository->tags does not see lightweight tags which is a bit confusing to command line users. I wonder if this is an undocumented quirk of git_tag_foreach, because git_tag_list does appear to return both annotated and lightweight tags, as evidenced in the libgit2 tag example.

What are your thoughts?

@jacquesg
Copy link
Owner

git_tag_foreach actually does return light-weight tags, Git::Raw is just ignoring it

STATIC int git_tag_foreach_cbb(const char *name, git_oid *oid, void *payload) {
    dSP;
    int rv = 0;
    git_object *tag;

    SV *cb_arg = NULL;
    git_foreach_payload *pl = payload;

    int rc = git_object_lookup(&tag, pl -> repo_ptr -> repository, oid, GIT_OBJ_ANY);
    git_check_error(rc);

    if (git_object_type(tag) != GIT_OBJ_TAG) {
        git_object_free(tag);

        return 0;
    }

The correct thing to do here is for a Git::Raw object to represent any type of tag, and add methods like is_lightweight etc. Let me see what I can do.

@schwern
Copy link
Contributor Author

schwern commented Sep 18, 2014

Thanks!

PS The urgency for Gitpan is considerably reduced, I used
Git::Raw::Reference to accomplish what I needed.

@jacquesg
Copy link
Owner

I'm actually looking right at this at the moment. Looks like the best I can do would be to improve the interface. A little bit weary of adding too much magic on top of libgit2, as it will just come back to bite in future (don't want to unnecessarily break compatibility in future).

@schwern
Copy link
Contributor Author

schwern commented Sep 18, 2014

What I would suggest is to leave Git::Raw as a thin wrapper around
libgit2. Then a separate module, using Git::Raw, makes it convenient.
This lets the work be separated, keeps Git::Raw flexible, and lets
multiple convenience implementations be written with different ideas
about what's convenient.

This offers a solution for lightweight tags. git_tag_foreach() and
git_tag_list() return all types of tags. If Git::Raw is supposed to be
a thin wrapper around libgit2 it shouldn't interfere and limit this
functionality. It should return all tags, as libgit2 does.

I see two ways forward...

  1. Return Tag and Reference/Commit objects and let the caller figure it
    out. This is what libgit2 does.

  2. Create a Git::Raw::Tag::Lightweight class which shares the interface
    of Git::Raw::Tag. Lightweight objects would be a delegation wrapper
    around Git::Raw::Commit and/or Reference objects. Most methods would
    work the same: lookup, owner, foreach, delete, id, name, target. Some
    would always return undef: message, tagger. is_lightweight() would be
    hard coded as appropriate.

I would suggest Git::Raw::Tag->foreach() is left as close to
git_tag_foreach() as possible, just return the objects as found as per

  1. 2 would be implemented as a separate convenience method, or left as
    an exercise for another module.

@jacquesg
Copy link
Owner

Agree on 1. I'm busy with the changes so Git::Raw::Tag->foreach will return either a Git::Raw::Commit or Git::Raw::Reference object. I think I'll allow you to optionally specify which type of tags you're interested in, all, annotated or light-weight.

2 is really user specific, not something I would like to add to Git::Raw.

@jacquesg
Copy link
Owner

This should be better now :)

@jacquesg
Copy link
Owner

I've created a new release (https://metacpan.org/release/JACQUESG/Git-Raw-0.45), see if it works better for you.

@jacquesg
Copy link
Owner

Thanks for your input, let me know if you have more suggestions/problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants