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

Consider accepting SHAs/tags rather than branches/tags #33

Open
MichaelXavier opened this issue Jun 13, 2017 · 4 comments
Open

Consider accepting SHAs/tags rather than branches/tags #33

MichaelXavier opened this issue Jun 13, 2017 · 4 comments

Comments

@MichaelXavier
Copy link

So I've been converting a project to use package sets and I needed a workflow for temporary changes to the package-sets repo in my fork while waiting on PRs. I noticed from the source code that its just shelling out to git and the git command being called happens to take a tag or a branch. If you try to specify a SHA it will tell you the SHA isn't a branch.

However, when you use a branch, the tool doesn't actually know how to get new updates for that branch. You have to actually delete the .package-sets subdir for your branch for it to update. I talked with someone in the IRC about it and they believe that the idea is that package sets are immutable (which seems like a good idea). If that's the case, tags fit the bill because they are effectively immutable, but branches seem like a misfeature. They are a moving target but aren't treated that way by psc-package. If we want to stick with immutability, it seems like a SHA is a better fit because it is basically analogous to a tag and the user would not infer mutability like they may with branches.

I think implementation wise, there's probably a git command to determine if a given string is a tag and if not, assume its a SHA and run a different git command depending. Thoughts?

@paf31
Copy link
Contributor

paf31 commented Jun 13, 2017

Ideally I'd like to restrict versions to tags or SHA hashes, for this reason. If there is a way to have Git verify a version is one of those, I think we should add it.

MichaelXavier added a commit to Soostone/psc-package that referenced this issue Jun 13, 2017
This could be a half-measure for purescript#33. This does not prevent usage of
master, but it arguably does behave how you'd expect: it always keeps
whatever reference you're using as a package set up to date, and
happens to also allow you to use a SHA in addition to a revision. I'm
going to give this a spin, but if we later wanted to disallow
branches, we could probably cobble together a git command that would
tell us if a tree-ish or whatever they call it is a branch or not.
@MichaelXavier
Copy link
Author

@paf31 https://github.com/purescript/psc-package/compare/master...Soostone:allow-revision?expand=1

I'm trying this out locally on a project and it appears to be working. It is a half-measure to this ticket in that it doesn't disallow branches. Instead, it allows set to be basically any tree-ish or whatever git calls it (a branch, tag, or SHA). It does this by not using the -b flag with checkout, but instead using fetch and reset --hard. You see this same technique I believe in git-based deploy tools like capistrano.

In my opinion this makes the tool conform to what the README says: "(usually a tag name, but a SHA is also valid)", which is not actually true for SHAs right now. The tool now runs a fetch on each dependency eagerly now because it isn't certain that nothing has changed. In the recommended case of tags or SHAs, unless someone does a force, nothing will have chaged, but in the case of branches, it will always track the latest on the branch, avoiding the weirdness of branches sorta working but only the first time. The down side is that this can be quite a few network calls that while normally paid just once when a ref is selected, is now paid whenver you call update.

My take is that once we do this, we'll be caught up with what the docs say. Next, if we can find the appropriate git incantations to tell us if something is a branch or not, we can make a breaking change, forbid the use of branches, and then the assumption that tags and SHAs are immutable will mean we could reinstate the policy of only fetching the first time.

@MichaelXavier
Copy link
Author

@paf31 Did you have any feelings on this? I see that there was a new release so I'm imagining this would have to be updated or a new approach would need to be taken.

@paf31
Copy link
Contributor

paf31 commented Jul 14, 2017

Yes, sorry, I meant to look at this and I missed it.

Do you think it's possible to reuse listRemoteTags, or git ls-remote to simplify this a bit?

Either way, could you please open a PR with what you have? I would like to address this.

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