Skip to content

Conversation

clatim
Copy link

@clatim clatim commented Oct 21, 2022

Using the latest version of the unicode-segmentation crate causes the tests to fail when run on exercism.

Updating the instructions to at least suggest a solution seems helpful.

Using the latest version of [the unicode-segmentation crate](https://crates.io/crates/unicode-segmentation) causes the tests to fail when run on exercism.

Updating the instructions to at least suggest a solution seems helpful.
@clatim clatim changed the title Update instructions.append.md Update instructions in reverse-string exercise. Oct 21, 2022
@bobahop
Copy link
Member

bobahop commented Nov 7, 2022

@petertseng A couple things about this, since I'm unfamiliar with how/where the external crates are configured for this.

Can the dependency for unicode-segmentation be set to something like unicode-segmentation = "1"? It seems it's been version 1.X for about six years, so I wouldn't think there are any backward compatibility issues, just that the runner won't except a version newer than the specific one it's configured for. I could be wrong, but maybe a more general version would alleviate the issue?

Or, if we need to have a note, might it be preferred as an '```exercism/note'?

@petertseng
Copy link
Member

Students will specify the external dependency in https://github.com/exercism/rust/blob/main/exercises/practice/reverse-string/Cargo.toml. Because this is an optional feature as described in https://github.com/exercism/rust/blob/main/exercises/practice/reverse-string/.docs/instructions.append.md, I presume that is why students specify the dependency, rather than the Rust track providing a Cargo.toml with the dependency already specified.

A more ideal solution would be that the test runner should expose precisely which crates and versions of those crates it allows, and then the student can refer to that list of crate versions to decide what to use. Of course, that ideal solution will take time to implement, so intermediate solutions could ease the pain in the meantime.

"Use a lower version" is too prone to the problem of having to guess what version, which can be frustrating. Better to advise the student to use a more general version specification.

@bobahop
Copy link
Member

bobahop commented Nov 7, 2022

Right, I know where a student specifies it. I don't know where the infrastructure specifies what it will allow from the student. I was thinking that if the infrastructure can specify something more general, then smaller version upgrades wouldn't be disallowed. I like the idea of doing that, if possible, or your ideal suggestion of informing the student what crates/versions are accepted.

I'm unfamiliar with the infrastructure, so I don't how much effort it would involve to allow a range of versions. I agree that just suggesting to use a lower version could be frustrating to the student if they keep picking one that isn't low enough to satisfy what's configured in the infrastructure. If configuring to accept a range of versions is relatively easy, I'm guessing that could be the intermediate solution before implementing the list of supported crates/versions?

@petertseng
Copy link
Member

The test runner adds these lines to Cargo.toml:

https://github.com/exercism/rust-test-runner/blob/main/bin/generate-registry.sh#L15-L21

I'll arbitrarily pick the first crate of https://github.com/exercism/rust-test-runner/blob/main/local-registry/supported_crates to demonstrate:

$ cargo search unicode_segmentation | head -1
unicode-segmentation = "1.10.0"    # This crate provides Grapheme Cluster, Word and Sentence boundaries according to Unicode Stan…

Of course, the version would be what cargo search would find at the time of the last successful build. According to https://github.com/exercism/rust-test-runner/commits/main, the last successful build was on 2022-05-11, so the versions available on that date are what is available to students

@bobahop
Copy link
Member

bobahop commented Nov 7, 2022

Thanks for fielding my novice questions re the test runner. I may be starting to see the light. I'm unfamiliar with bash, so I don't know how much of a hassle it would be in it to change

unicode-segmentation = "1.10.0"    # This crate provides...

to

unicode-segmentation = "1"    # This crate provides...

If that were doable, am I wrong in thinking that it would support all versions from 1 until 2? I'm looking at the The Cargo Book,

@petertseng
Copy link
Member

Ultimately it depends on the behaviour of the tool we use to build the local registry. Since I don't know anything about https://github.com/dhovart/cargo-local-registry/blob/master/src/main.rs, it is not possible for me to guess what effect that proposed change will have.

Naturally, because the only reason for the above statement is that I don't know, it means we should remember not to let that ignorance stifle exploration.

@senekor
Copy link
Contributor

senekor commented Jul 7, 2023

I don't think it's possible to allow a broader range of versions to be accepted. If the manifest provided by the student specifies a newer version than the one in the registry, compilation should definitely fail from cargo's perspective. After all, the person who wrote the manifest may rely on features or bug fixes from the newer version.

We can't really do anything about this at the time of building the registry. Building with the latest version of a library provides the best compatibility, but it cannot provide compatibility with future versions.

Thoughts about alternatives:

  1. Putting notices in exercise instructions seems overkill. Every exercise may use dependencies, so every exercise would need such a notice.
  2. We could minimize the problem by keeping the registry as up-to-date as possible. (e.g. cron job on github actions which triggers a rebuild of the test runner whenever a new crate version is available)
  3. We could provide the user with a better error message then cargo's when it does happen. (maybe via our analyzer?)

I like number 2, it would be the biggest step for a smoother user experience. It would take a bit of work though.

@senekor
Copy link
Contributor

senekor commented Jul 21, 2023

This problem should be fixed if exercism/rust-test-runner#77 is merged.

@senekor
Copy link
Contributor

senekor commented Sep 10, 2023

The test runner now has pinned versions and dependabot bumps them automatically every week. The likelihood of this problem occurring again should be minimized.

@senekor senekor closed this Sep 10, 2023
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

Successfully merging this pull request may close these issues.

4 participants