Skip to content

Crate and feature lists in wasm CI loops can easily go stale #1988

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

Open
EliahKagan opened this issue May 4, 2025 · 3 comments
Open

Crate and feature lists in wasm CI loops can easily go stale #1988

EliahKagan opened this issue May 4, 2025 · 3 comments
Labels
acknowledged an issue is accepted as shortcoming to be fixed help wanted Extra attention is needed

Comments

@EliahKagan
Copy link
Member

Current behavior 😯

The wasm job definition in ci.yml contains steps that loop over various subsets of crates and features. One of the lists of crates is rather long:

- name: crates without feature toggles
run: |
set +x
for name in gix-actor gix-attributes gix-bitmap gix-chunk gix-command gix-commitgraph gix-config-value gix-date gix-glob gix-hash gix-hashtable gix-mailmap gix-object gix-packetline gix-path gix-pathspec gix-prompt gix-quote gix-refspec gix-revision gix-traverse gix-url gix-validate; do
(cd -- "$name" && cargo build --target "$TARGET")
done

For that list especially, but also other lists of crates and of features, it looks like no mechanism is in place to keep the lists updated or to automatically determine if they are up to date. For example, a crate may gain or lose feature toggles across versions.

Expected behavior 🤔

I don't know if it would make sense to dynamically generate those lists of crates, but even if not, I think it would be good to have some way to verify if they are still accurate to their descriptions. This could be a new CI job, or a new justfile recipe, or a new justfile recipe that is called in a new CI job.

(I don't think the check would depend on which wasm target is used, which is why I do not suggest doing it in a new step of that job, which would cause it to be done twice. That could be done if it turns out to be clearer or more elegant overall, though.)

Git behavior

Not applicable.

Steps to reproduce 🕹

For the general issue, see above.

As for the question of whether the lists are current as of now, I have not (yet?) investigated that.

EliahKagan added a commit to EliahKagan/gitoxide that referenced this issue May 4, 2025
- Use a `bash` array for the long list of crates without feature
  toggles, to make clear what crates are covered in this loop, and
  so any changes to the list produce clearer diffs, and to improve
  the general readability of the workflow.

- In each loop, rename the variable from `name` to `crate`, to make
  the meaning clearer. The other significant kind of loop in this
  job is over `feature`, so this makes the contrast clear. (Other
  loops over array items in other jobs in the workflow, where the
  general variable name `name` could be used, already instead use
  the more specific variable names `package` and `cmd`.)

This may slightly mitigate GitoxideLabs#1988 in that it makes it easier for the
big list to be manually inspected, but it does not fix that issue.
@Byron Byron added the acknowledged an issue is accepted as shortcoming to be fixed label May 5, 2025
@Byron
Copy link
Member

Byron commented May 5, 2025

Thanks for bringing this up!

While the current approach of hardcoding a list in a somewhat inaccessible place will detect when WASM compatibility is lost, there is no way to determine if the list of crates is complete just yet.

Thus far, this was acceptable as WASM is clearly in its early stages and there is no known product to support just yet.

In any case, something I could imagine is a justfile target that probes for WASM compatibility and that writes that list to a file. That way it could be run occasionally to update the list which is then used by CI instead of hardcoding the list into the workflow description. Maybe there are better ways though, like a job which instead tries every crate, keeps all the successful ones, and compares the final list with one that is stored to fail if they don't match. That way, both loss and gain of compatibility would be detected and one would be forced to act.

@Byron Byron added the help wanted Extra attention is needed label May 5, 2025
@EliahKagan
Copy link
Member Author

Is the categorization based on which crates are believed to work (or build) on WASM? Or is it based on which crates are believed to have particular features? Or based on which crates are believed to have particular features that work on WASM? I was thinking of it as being the latter, but I now realize I am not sure that is correct. Perhaps it is simpler than I think?

If this were just a matter of keeping track of which crates are expected to build, then it could be done with a technique similar to the comparison in test-fixtures-windows, except for lists of which crates succeed under cargo check rather than lists of which test cases pass under cargo nextest run --workspace. But it seems to me that this is more than a binary categorization and that there could be other subtleties.

@Byron
Copy link
Member

Byron commented May 5, 2025

Is the categorization based on which crates are believed to work (or build) on WASM? Or is it based on which crates are believed to have particular features? Or based on which crates are believed to have particular features that work on WASM? I was thinking of it as being the latter, but I now realize I am not sure that is correct. Perhaps it is simpler than I think?

This list was created by manually trying to compile each crate (at the time) to WASM, and those that worked were added to the list to assure they don't (unnecessarily) stop working. It was meant as a first step to more fully support WASM.

But it seems to me that this is more than a binary categorization and that there could be other subtleties.

I think this would work, absolutely, and would be an improvement over what's here today. Since it's WASM, and there are multiple possible WASM'ish targets to choose from, I am sure there is much more to it. Generally, without a known downstream consumer this is really just best-effort, or a blind stab that ideally is cheap to maintain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledged an issue is accepted as shortcoming to be fixed help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants