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

tools-in-path should use framework version, not sdk version. #333

Merged
merged 1 commit into from
Jan 30, 2024
Merged

tools-in-path should use framework version, not sdk version. #333

merged 1 commit into from
Jan 30, 2024

Conversation

nicrowe00
Copy link
Contributor

@nicrowe00 nicrowe00 commented Jan 29, 2024

tools-in-path is failing on dotnet 8.0.0 as in that version of dotnet the sdk version is written as "8.0.1" whereas the framework is 8.0.0. This causes an issue when installing dotnet-ef as it will go by the sdk version and there is a newer version of dotnet-ef for dotnet 8.0.101, causing a compatibly issue when used with dotnet 8.0.0 as it will automatically download the newest version of dotnet-ef for 8.0.1 . Due to this it would be better to download the version of dotnet-ef that matches the framework version of dotnet, rather than the sdk.

Since the latest release of dotnet 8, 8.0.101, tools-in-path has been failing
as even though there is a version of dotnet-ef for 8.0.1, it expects the dotnet framework installed to also be 8.0.1,
whereas the framework used in dotnet 8.0.1 is still 8.0.0 which causes a compatibility error and a test failure.
Instead of installing the version of dotnet-ef that matches the installed sdk version,
the test should download the version of dotnet-ef that matches the installed framework.
@nicrowe00 nicrowe00 requested review from omajid and tmds January 29, 2024 10:42
@nicrowe00
Copy link
Contributor Author

CI errors are unrelated.

@tmds
Copy link
Member

tmds commented Jan 29, 2024

it expects the dotnet framework installed to also be 8.0.1, whereas the framework used in dotnet 8.0.1 is still 8.0.0

I'm not sure what this means. What does the error look like?

Also, from the command we're using to install the tool, it seems we'd accept anything that starts with an 8.?

@nicrowe00
Copy link
Contributor Author

@tmds Apologies, I made an error in my commit message. I've updated the message. Below is the error:

Welcome to .NET 8.0!
---------------------
SDK Version: 8.0.100

----------------
Installed an ASP.NET Core HTTPS development certificate.
To trust the certificate, view the instructions: https://aka.ms/dotnet-https-linux

----------------
Write your first app: https://aka.ms/dotnet-hello-world
Find out what's new: https://aka.ms/dotnet-whats-new
Explore documentation: https://aka.ms/dotnet-docs
Report issues and find source on GitHub: https://github.com/dotnet/core
Use 'dotnet --help' to see available commands or visit: https://aka.ms/dotnet-cli
--------------------------------------------------------------------------------------
Skipping NuGet package signature verification.
You can invoke the tool using the following command: dotnet-ef
Tool 'dotnet-ef' (version '8.0.1') was successfully installed.
+ dotnet ef --version
You must install or update .NET to run this application.

App: /root/.dotnet/tools/dotnet-ef
Architecture: x64
Framework: 'Microsoft.NETCore.App', version '8.0.1' (x64)
.NET location: /usr/lib64/dotnet

The following frameworks were found:
  8.0.0 at [/usr/lib64/dotnet/shared/Microsoft.NETCore.App]

Learn more:
https://aka.ms/dotnet/app-launch-failed

To install missing framework, download:
https://aka.ms/dotnet-core-applaunch?framework=Microsoft.NETCore.App&framework_version=8.0.1&arch=x64&rid=rhel.8-x64&os=rhel.8
Process Exit Code: 150

@omajid
Copy link
Member

omajid commented Jan 29, 2024

Ah, so we need to ask for a dotnet-ef with the version matching the runtime version? Makes sense.

@tmds
Copy link
Member

tmds commented Jan 29, 2024

The error is more clear now.

The test fails because upstream runtime version 8.0.1 is available, and the environment that the test is running on is still providing 8.0.0.

It's not bad that something in our test suite fails because of this.

Are there other tests failing?

@nicrowe00
Copy link
Contributor Author

There are other tests failing due to this that use dotnet 8.0.0.

@tmds
Copy link
Member

tmds commented Jan 29, 2024

I'm fine updating this test so it works against older runtime versions.

It would be nice to also add a new test that fails when the runtime version tested against is older than what is available publically. We can create an issue to add such a test.

@nicrowe00 nicrowe00 closed this Jan 29, 2024
@nicrowe00 nicrowe00 reopened this Jan 29, 2024
@omajid
Copy link
Member

omajid commented Jan 29, 2024

It would be nice to also add a new test that fails when the runtime version tested against is older than what is available publically. We can create an issue to add such a test.

That's release-version-sane. It is also the only test that's supposed to fail on a version mismatch. We skip that in PR tests in this repo to reduce noise - because we can't control whether that test passes or fails.

@tmds
Copy link
Member

tmds commented Jan 29, 2024

That's release-version-sane. It is also the only test that's supposed to fail on a version mismatch. We skip that in PR tests in this repo to reduce noise - because we can't control whether that test passes or fails.

Interesting. Is release-version-sane failing or being skipped on the environment you're trying to get tools-in-path to pass, @nicrowe00?

@nicrowe00
Copy link
Contributor Author

@tmds release-version-sane is failing on the environment when dotnet 8 is used.

@tmds
Copy link
Member

tmds commented Jan 30, 2024

@tmds release-version-sane is failing on the environment when dotnet 8 is used.

Good! This is the expected behavior.

I was asking what test failed in #333 (comment) to know what tests will continue to catch this, and which others we may also need to update.

Are there other tests failing than tools-in-path and release-version-sane due to using the older runtime?

@nicrowe00
Copy link
Contributor Author

No, no other tests are failing due to using an older runtime. tools-in-path and release-version-sane are the only tests affected by this.

@nicrowe00 nicrowe00 merged commit ca4d94a into redhat-developer:main Jan 30, 2024
38 of 50 checks passed
@omajid
Copy link
Member

omajid commented Jan 30, 2024

release-version-sane is failing on the environment when dotnet 8 is used.

I don't understand this comment. I think I am forgetting or missing some context. Can you tell us more?

@nicrowe00
Copy link
Contributor Author

@omajid In the environment the test suite is running on, release-version-sane fails when dotnet 8 is installed because it is not the latest version of dotnet 8. dotnet 6 and 7 are also tested on this environment, and release-version-sane does not fail on these versions.

@omajid
Copy link
Member

omajid commented Jan 30, 2024

That seems strange. Do you understand why that happens? What's unique about that environment or .NET 8?

@nicrowe00
Copy link
Contributor Author

It's happening because the environment is built from a RHEL image and the version of RHEL used contains the latest release of dontet 6 and 7 but not the latest release of dotnet 8.

@omajid
Copy link
Member

omajid commented Jan 30, 2024

I see. Thank you for clearing this up.

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.

3 participants