purescript / psc-package

A package manager for PureScript based on package sets
https://psc-package.readthedocs.io
Other
228 stars 45 forks source link

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

Open MichaelXavier opened 7 years ago

MichaelXavier commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

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.