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

UX: Shards requires tags to start with "v" #521

Open
z64 opened this issue Aug 18, 2021 · 15 comments · May be fixed by #626
Open

UX: Shards requires tags to start with "v" #521

z64 opened this issue Aug 18, 2021 · 15 comments · May be fixed by #626

Comments

@z64
Copy link

z64 commented Aug 18, 2021

I recently went to restrict a dependency to a specific version:

dependencies:
  taskbox:
    git: git@git.sr.ht:~z64/taskbox
    version: 0.2.0

and received:

$ shards -v install
Resolving dependencies
git ls-remote --get-url origin
Fetching git@git.sr.ht:~z64/taskbox
git fetch --all --quiet
git tag --list --column=never
Unable to satisfy the following requirements:

- `taskbox (0.2.0)` required by `shard.yml`
Failed to resolve dependencies

so I did some checking:

taskbox $ git tag --list --column=never
0.1.0
0.1.1
0.2.0

taskbox $ cat shard.yml
name: taskbox
version: 0.2.0

Seeing as it wasn't working even when everything shown to me says "0.2.0" so far, I checked the source:

$ rg 'VERSION_TAG' src/
src/resolvers/git.cr
209:        .compact_map { |tag| Version.new($1) if tag =~ VERSION_TAG }

src/config.cr
13:  VERSION_TAG           = /^v(\d+[-.][-.a-zA-Z\d]+)$/

and it would appear there is a hard requirement that tags start with "v".


The docs have this to say about git:

git
A Git repository URL (string).
The URL may be any protocol supported by Git, which includes SSH, GIT and HTTPS.
The Git repository will be cloned, the list of versions (and associated shard.yml) will be extracted from Git tags (e.g., v1.2.3).
One of the other attributes (version, tag, branch or commit) is required. When missing, Shards will install the HEAD refs.
Example: git: git://git.example.org/crystal-library.git

If I am not missing something, if the "e.g., v1.2.3" was supposed to convey this requirement, I think it would be nice to reword this section to make it more clear - or even better, an improved error message.

Personally, I'm not sure that this requirement is necessary in the first place, but I don't really mind as long as the above UX could be smoothed over.

@asterite
Copy link
Member

Yeah... I don't know why there's this v prefix requirement. I wish we would remove it. I don't understand what's its purpose... 😞

@Sija
Copy link
Contributor

Sija commented Aug 18, 2021

We could make it optional, WDYT?

@straight-shoota
Copy link
Member

This is technically documented in https://github.com/crystal-lang/shards#usage, but I agree that this documentation is not very accessible and thus not user friendly.

We currently have manual pages for the shards command and for shard.yml, but not for the integration and expected interfaces with source code management systems (which we should definitely take on as #458 is approaching).

On the question itself: We could add support for non-prefixed version tags. But I don't see any benefit from that. It just complicates the resolver.
I'd say it's similar to aliases: Why would we need two different ways to tag versions, when one suffices?

This was previously discussed in #5 and (more recently and elaborate) in #108 (comment) ff.

@z64
Copy link
Author

z64 commented Aug 24, 2021

I will reiterate a little bit: In my report, I happen to be the owner of the shard that I'm using, and I deliberately ran the command with -v for the sake of the report. A downstream user is going to see this:

$ shards install
Resolving dependencies
Fetching git@git.sr.ht:~z64/taskbox
Unable to satisfy the following requirements:

- `taskbox (0.2.0)` required by `shard.yml`
Failed to resolve dependencies

How do they know what to do next? I think a product of this issue should be improving this story. Even if the problem is "the author of the shard did not read the documentation", it could be that the first person to deal with this issue is a downstream user who needs to submit a good bug report.

Maybe the smallest step is changing the hints so that shards itself says "v1.2.3" everywhere, if that is so intrinsic to its behavior:

- `taskbox (v0.2.0)` required by `shard.yml`

Back on requiring v: I don't follow the listed arguments why shards needs to be opinionated about this. "complicating the resolver" doesn't seem to mean anything to the end-user.

I'd say it's similar to aliases: Why would we need two different ways to tag versions, when one suffices?

I don't think this shoe really fits the argument; anyone who is tagging versions is only going to pick a single way of writing them. I could see it if someone were to tag both v1.2.3 and 1.2.3... but I imagine mixing styles to be much more rare, and that would be sensibly a hard error from shards that the source was ambiguous. Easily fixed by the author, easily reported by a user.

And with respect to the Crystal language's take on aliases, it (crystal) still does not impede you from doing them.

If I were writing shards, I would look no further than semver as a guide: (emphasis mine)

Is “v1.2.3” a semantic version?

No, “v1.2.3” is not a semantic version. However, prefixing a semantic version with a “v” is a common way (in English) to indicate it is a version number. Abbreviating “version” as “v” is often seen with version control. Example: git tag v1.2.3 -m "Release version 1.2.3", in which case “v1.2.3” is a tag name and the semantic version is “1.2.3”.

which acknowledges it as an informal practice WRT the spec. So if there is an "one-way" opinion to be had, I think it would be that v is not allowed.

But I'm digging for an argument here - this seems like too small of a detail to corral users with.

@straight-shoota
Copy link
Member

I think your argument can actually be turned the other way around:

The error message about satisfying `taskbox (0.2.0)` required by `shard.yml` is completely correct. The resolver is unable to find the version 0.2.0.
v0.2.0 is just the respective git tag, not the version. So I don't think it would make sense to present it there.

Maybe what would help in this case is a shards command for displaying available versions of a shard. So instead of listing git tags, you could run shards versions taskbox for example, which would give an empty output.

anyone who is tagging versions is only going to pick a single way of writing them.

And thus there's no reason to provide multiple ways for writing them.

At this point, I think we don't even need a strong argument for v prefix being better than no prefix. Just having thousands of shard releases represented as git tags with v prefix makes changing the naming scheme very costly.

@z64
Copy link
Author

z64 commented Aug 24, 2021

I think your argument can actually be turned the other way around:
The error message about satisfying taskbox (0.2.0) required by shard.yml is completely correct. The resolver is unable to find the version 0.2.0.
v0.2.0 is just the respective git tag, not the version. So I don't think it would make sense to present it there.

That wasn't an argument really; I was just saying the error message is entirely unhelpful when thinking about the root of the problem, and that was a small suggestion to give some kind of hint that a tag with v* wasn't present, short of a message that fully explains the issue. i.e.

No tag listed as "v1.2.3" found, but "1.2.3" is present. Perhaps it is incorrectly tagged?

instead of flatly

Unable to satisfy the following requirements:

- `taskbox (0.2.0)` required by `shard.yml`
Failed to resolve dependencies

which tells you nothing about how to proceed.


And thus there's no reason to provide multiple ways for writing them.

I would not say that shards provides this, when the process of tagging does not directly involve shards at all. If there was a shards publish command that takes care of this for you, that would be more apt. But I see no reason that shards needs to be the arbiter here over something so trivial.

There are also ecosystems that do not care about v. Rust cargo does not require v, and plenty of the most popular crates do not use v when tagging.

Sure the majority might use v. But enforcing v only causes friction for those who don't want to use it, and no benefit for those who do. It's just one character - let people do what they want. shards is entirely capable of figuring it out for them.

syeopite added a commit to syeopite/lens that referenced this issue Sep 1, 2021
Version tags needs to be prefixed with `v` but shards.yml uses the
numerical values.

Related: crystal-lang/shards#521
@z64
Copy link
Author

z64 commented Apr 19, 2024

I just lost another 2 hours of time because shards could not tell me that the version I specified does not exist, regardless of using verbose flags. We are just going to stop using shards at this point - there are simply too many UX issues and ecosystem footguns (some related to shards, others not) to justify using it. The "happy path" is fine, but shards is basically useless if anything goes wrong - this v thing, unknown version, the upstream repo disappears, the host site is down, SSL failure etc. are all things I've collectively lost so much time on by now because shards simply couldn't tell me what went wrong and I have to manually pull and run the git commands myself. I implore the team to focus on this.

@devnote-dev devnote-dev linked a pull request Apr 19, 2024 that will close this issue
@ysbaddaden
Copy link
Contributor

@z64 Your problem is error reporting in many scenarios, not about tags having a v. Please report issues with explanation of what went wrong and how shards could have helped you. Pull requests are also welcome.

@z64
Copy link
Author

z64 commented Apr 19, 2024

@ysbaddaden I have acknowledged that in just about every single message from me in this thread. That is all I am asking for is better errors at the least. I have also suggested changes to how shards works that I would prefer, but the error messages are all what seriously need improving in many cases to save me the time that I've lost debugging these things.

In my last message:

I've collectively lost so much time on by now because shards simply couldn't tell me what went wrong

Just replicate the steps I took in my OP, or any of the things I listed in my last message, and make them give a useful error / suggestion.

@beta-ziliani
Copy link
Member

I wouldn't go as far as to say which tags are present, but suffices to say something along the lines "Please check the tag v#{VERSION} exists in the repository".

@devnote-dev
Copy link

@beta-ziliani In any case this shouldn't happen, as @z64 already stated the "v" prefix is not standard and is not a hard requirement by other package managers, and there's no real reason for Crystal to not follow this practice. Furthermore, this prefix is not a requirement in the VERSION_REFERENCE regex that is also used in the Git resolver. Had this been enforced here too, I'm certain it would have been fixed before this issue was even created (for reference, this code is over 6 years old from when @ysbaddaden committed it).

@z64
Copy link
Author

z64 commented Apr 19, 2024

I wouldn't go as far as to say which tags are present, but suffices to say something along the lines "Please check the tag v#{VERSION} exists in the repository".

Exactly, though I think its a little too subtle. Above I suggested displaying a hint like:

No tag listed as "v1.2.3" found, but "1.2.3" is present. Shards requires tags to begin with "v".
Perhaps it is incorrectly tagged?

Coupled with a shards release type command to make it harder for this to happen in the first place, and users don't have to fork or chase down maintainers, would be a fantastic improvement.

@z64
Copy link
Author

z64 commented Apr 19, 2024

@devnote-dev To be clear on my stance, I am fine with shards requiring the prefix, IF the tooling helps you when things are incorrectly set up, or even better - prevent invalid configurations from being created in the first place.

If that can't be done, then I think shards needs to relax the requirements. Either way is fine by me.

@devnote-dev
Copy link

@z64 That's fine, I've chosen to relax the requirement in my PR anyway as it's a simpler solution, tracing typings and exceptions in Shards' codebase has proven to be tedious.

@ysbaddaden
Copy link
Contributor

As far as I recall VERSION_REFERENCE isn't the reference format of a version, it's meant to parse Git refs that look like a version —frankly, I don't recall why we need that.

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

Successfully merging a pull request may close this issue.

7 participants