Skip to content

Support "v" prefixes in prerelease module version constraints#2124

Merged
apparentlymart merged 7 commits into
opentofu:mainfrom
AYM1607:prerelease-v-prefix
Nov 6, 2024
Merged

Support "v" prefixes in prerelease module version constraints#2124
apparentlymart merged 7 commits into
opentofu:mainfrom
AYM1607:prerelease-v-prefix

Conversation

@AYM1607
Copy link
Copy Markdown
Contributor

@AYM1607 AYM1607 commented Oct 31, 2024 •
edited
Loading

Resolves #2117

Target Release

1.9.0

Checklist

  • I have read the contribution guide.
  • I have not used an AI coding assistant to create this PR.
  • I have written all code in this PR myself OR I have marked all code I have not written myself (including modified code, e.g. copied from other places and then modified) with a comment indicating where it came from.
  • I (and other contributors to this PR) have not looked at the Terraform source code while implementing this PR.

Go checklist

  • I have run golangci-lint on my change and receive no errors relevant to my code.
  • I have run existing tests to ensure my code doesn't break anything.
  • I have added tests for all relevant use cases of my code, and those tests are passing.
  • I have only exported functions, variables and structs that should be used from other packages.
  • I have added meaningful comments to all exported functions, variables, and structs.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Oct 31, 2024

Reminder for the PR assignee: If this is a user-visible change, please update the changelog as part of the PR.

@AYM1607 AYM1607 mentioned this pull request Oct 31, 2024
Closed
@AYM1607 AYM1607 force-pushed the prerelease-v-prefix branch from 10afb44 to 4aff810 Compare November 1, 2024 00:37
@AYM1607 AYM1607 marked this pull request as ready for review November 1, 2024 00:44
@AYM1607 AYM1607 requested a review from a team as a code owner November 1, 2024 00:44
@AYM1607 AYM1607 changed the title adds tests for module prerelease version constraints with a v prefix Support "v" prefixes in prerelease module version constraints Nov 1, 2024
@AYM1607 AYM1607 force-pushed the prerelease-v-prefix branch from fcbf694 to 30595ea Compare November 1, 2024 02:15
AYM1607
AYM1607 commented Nov 5, 2024
View reviewed changes
Comment thread internal/initwd/module_install.go
@AYM1607 @apparentlymart
adds tests for module prerelease version constraints with a v prefix
b23684e 
Signed-off-by: AYM1607 <u.g.a.mariano@gmail.com>
@AYM1607 @apparentlymart
Use the hashicorp/go-version for handling pre-release version constra…
8e99ad0 
…ints during module installation. Adds more tests.

Signed-off-by: AYM1607 <u.g.a.mariano@gmail.com>
@AYM1607 @apparentlymart
Add prerelease test with multiple constraints.
4bc1ef0 
Signed-off-by: AYM1607 <u.g.a.mariano@gmail.com>
@AYM1607 @apparentlymart
Revert back to the previous implementation.
12d4a90 
Signed-off-by: AYM1607 <u.g.a.mariano@gmail.com>
@AYM1607 @apparentlymart
Strip "v" prefix from constraints when parsing by apparentlymart/go-v…
94fa9ed 
…ersions fails.

Signed-off-by: AYM1607 <u.g.a.mariano@gmail.com>
@AYM1607 @apparentlymart
Update logging wording.
9f410de 
Signed-off-by: AYM1607 <u.g.a.mariano@gmail.com>
apparentlymart
apparentlymart previously approved these changes Nov 6, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@apparentlymart apparentlymart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this @AYM1607, and in particular for your patience while we worked through different possible approaches to fix it.

Although what we've ended up here is definitely not what we would've written if we were writing this code new today, I think it represents the best compromise to narrowly fix just the bug that was reported in #2117 while minimizing the risk of changing OpenTofu's interpretation of any other version constraint string.

Once this is merged (which I intend to do once the checks all show as passing) I will make a new issue to represent both the existing technical debt and the new technical debt we've accepted in this PR, so that we'll have a record of it and consider how we might make a more comprehensive improvement to this component in future.

Thanks again!

@apparentlymart
initwd: Commentary on version constraint parsing compromises
a8388ef 
Due to some historical technical debt in this area we are intentionally
accepting a rather hacky but narrowly-scoped solution to a bug that
prevented selection of prerelease versions of modules when a version
constraint was written with a "v" symbol before the version selection,
such as in "=v1.0.0-beta1".

This commit just records some commentary about the decision for the benefit
of a future maintainer that is likely to wonder why this code is written
the way it is, and (assuming GitHub outlives these comments) link back to
the discussion that motivated it.

Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
@apparentlymart apparentlymart merged commit d396502 into opentofu:main Nov 6, 2024
@AYM1607 AYM1607 deleted the prerelease-v-prefix branch November 6, 2024 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@apparentlymart apparentlymart apparentlymart approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

tofu init cannot find module version with a "v" prefix

2 participants

@AYM1607 @apparentlymart