Skip to content

Perf: Workaround a nuget restore issue for MicroBenchmarks.csproj #3168

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

Closed
wants to merge 4 commits into from

Conversation

radical
Copy link
Member

@radical radical commented Jul 27, 2023

error NU1102: Unable to find package System.Text.Json with version (>= 8.0.0-rc.1.23375.7)
error NU1102:   - Found 1290 version(s) in dotnet8 [ Nearest version: 8.0.0-rc.1.23375.4 ]
error NU1102: Unable to find package System.Security.Cryptography.Xml with version (>= 8.0.0-rc.1.23375.7)
error NU1102:   - Found 1290 version(s) in dotnet8 [ Nearest version: 8.0.0-rc.1.23375.4 ]

Note the difference between the referenced version, and the one
available in the feed.

These packages are getting added as transitive references.

The hypothesis is that the direct package references have version string
as 8.0.*-* which select the latest available version.
- but those packages transitively reference others for whom the
latest version might not have been published to nuget yet.

To work around this, we add explicit references to such packages, so
they can get the versions selected from the ones in the feed.

Issue: #3164

```
error NU1102: Unable to find package System.Text.Json with version (>= 8.0.0-rc.1.23375.7)
error NU1102:   - Found 1290 version(s) in dotnet8 [ Nearest version: 8.0.0-rc.1.23375.4 ]
error NU1102: Unable to find package System.Security.Cryptography.Xml with version (>= 8.0.0-rc.1.23375.7)
error NU1102:   - Found 1290 version(s) in dotnet8 [ Nearest version: 8.0.0-rc.1.23375.4 ]
```

Note the difference between the referenced version, and the one
available in the feed.

These packages are getting added as transitive references.

The hypothesis is that the direct package references have version string
as `8.0.*-*` which select the latest available version.
    - but those packages transitively reference others for whom the
      latest version might not have been published to nuget yet.

To work around this, we add explicit references to such packages, so
they can get the versions selected from the ones in the feed.

Issue: dotnet#3164
lewing
lewing previously approved these changes Jul 27, 2023
@lewing
Copy link
Member

lewing commented Jul 27, 2023

I'm in favor of seeing if this resolves the restore problem and gets things building again, it has some unfortunate issues around what will actually get benchmarked but those already exist with other wildcards currently being used

radical added a commit to radical/runtime that referenced this pull request Jul 27, 2023
@radical
Copy link
Member Author

radical commented Jul 27, 2023

@radical
Copy link
Member Author

radical commented Jul 27, 2023

Though something to keep in mind is that the issue doesn't show up on every run, so if we don't want to merge, then we might have to run this a few times.

Copy link
Member

@ericstj ericstj left a comment

Choose a reason for hiding this comment

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

What's the theory behind why this would even work? If it fixes anything at all it means we'd be testing in a torn configuration. I don't think we want to do that. Instead we'd just want to retry and hope that more packages were pushed.

@radical
Copy link
Member Author

radical commented Jul 27, 2023

What's the theory behind why this would even work? If it fixes anything at all it means we'd be testing in a torn configuration. I don't think we want to do that. Instead we'd just want to retry and hope that more packages were pushed.

The retry might not help, because IIUC the package publishing can take some time. Maybe we should move the references to maestro based versions.

@cincuranet
Copy link
Contributor

I'm working on the Maestro version.

@radical
Copy link
Member Author

radical commented Jul 27, 2023

What's the theory behind why this would even work?

The hypothesis is that the direct package references have version string
as 8.0.- which select the latest available version.

  • but those packages transitively reference others for whom the
    latest version might not have been published to nuget yet.
    To work around this, we add explicit references to such packages, so
    they can get the versions selected from the ones in the feed.

If it fixes anything at all it means we'd be testing in a torn configuration. I don't think we want to do that.

👍

The comment #3164 (comment) suggests that we should be using the maestro versions, which @cincuranet is working on. Considering that, I'm closing this PR.

@radical radical closed this Jul 27, 2023
@radical radical deleted the add-direct-nuget-refs branch July 27, 2023 15:47
@lewing
Copy link
Member

lewing commented Jul 27, 2023

What's the theory behind why this would even work? If it fixes anything at all it means we'd be testing in a torn configuration. I don't think we want to do that. Instead we'd just want to retry and hope that more packages were pushed.

As I mentioned in my comment it, my view is we're already potentially testing a torn configuration for any of the wildcard packs that are listed explicitly (there are a lot). I agree we need a better long term solution but I'm also extremely frustrated that I'm getting zero perf data as we approach rc1.

@LoopedBard3
Copy link
Member

I agree we need a better long term solution but I'm also extremely frustrated that I'm getting zero perf data as we approach rc1.

Could you give some specifics for either what data is missing or where the data is missing? I want to make sure that there is not another issue occuring that is being overshadowed by this issue. Thanks!

@lewing
Copy link
Member

lewing commented Jul 27, 2023

#3181
#3180

are a couple

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

Successfully merging this pull request may close these issues.

5 participants