-
Notifications
You must be signed in to change notification settings - Fork 420
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
updated to Roslyn 2.7.0 #1132
updated to Roslyn 2.7.0 #1132
Conversation
@filipw Can you also update the changelog? Makes it a little easier for me when we do a release 😄 . LGTM other than that. |
ah thanks for spotting 👍 |
@filipw looks like mono is not happy. @DustinCampbell is this using the embedded mono build to run tests or the mono installed on travis? Either way an error like |
ugh you are right. It worked fine on my Windows box in VS Code, but on macOS I see this:
|
Actually, I was on an older Mono (5.4.0) when it failed for me. After upgrading to 5.8.0 everything works fine. |
We should reach out to the Mono folks to help us pull bits for our embedded Mono automatically. In the meantime, yes, we should update this. I can take a look later this week. |
@filipw: The build appears to have failed with a similar stack using Mono 5.8.0. Maybe try setting Mono to "latest" or "beta" to see if this problem is in the latest Mono |
FWIW, it appears that this same issue was found with MonoDevelop as well: mono/monodevelop#3693. @KirillOsenkov also ran into the same problem (dotnet/roslyn#23520) and it appears to have been fixed in Mono. I suspect we'll need to use a Mono newer than 5.8.0 though. |
Yes, you'll need Mono >= 5.8.0.130 to avoid hitting this bug. |
Thanks @KirillOsenkov! Looks like we're just shy of that version: 😄
|
Hmm, I may be misremembering, it could be that 129 is good enough. If you're not seeing the crash on 5.8.0.129 then it's that one :) For sure 5.8.0.0 was affected though. |
So on my Mac, when using But the tests run against the local Mono from the |
Ah, ok. That makes sense. I've actually pushed new Mono runtimes and frameworks to blob storage. I haven't verified them manually yet, but try updating https://github.com/OmniSharp/omnisharp-roslyn/blob/master/build.json with the following:
|
thanks! this did the trick on my Mac - the test suite passes there without issues. It also fixed the issue on travis - on Mac agent.
So somehow there appears to be a corlib version mismatch somewhere. The only thing I could think of is that the "global" Mono we install on Travis ( |
I bet I grabbed the wrong Linux runtimes. I'll check when I get into the office. |
ah that could be that indeed! Could also be that they packaged a wrong DLL 😊 |
FWIW, the Linux builds of |
Also, I've already updated the bits in blob storage. I'm going to submit a PR with the new Mono runtimes to ensure it passes CI. In the meantime, if you like, you can update them in your PR to ensure everything looks good. |
Here's the PR I submitted to move us to Mono |
@DustinCampbell thanks a lot - it's green now 🍻 |
.travis.yml
Outdated
@@ -6,7 +6,7 @@ env: | |||
- secure: m2PtYwYOhaK0uFMZ19ZxApZwWZeAIq1dS//jx/5I3txpIWD+TfycQMAWYxycFJ/GJkeVF29P4Zz1uyS2XKKjPJpp2Pds98FNQyDv3OftpLAVa0drsjfhurVlBmSdrV7GH6ncKfvhd+h7KVK5vbZc+NeR4dH7eNvN/jraS//AMJg= | |||
- secure: EA2fP5ymar2/ZM2G4cyP3FzK437zv1wP03AgUPCcQgke8Z5oG8Y5U632AzxBeeaqGAxHEi5Ewbq2A8N93pN+xAsoGlQf+AdLJROCeo7gy9O589Z8tmp/vAdMzZzyNhKi7SUSxOJ/TIDzLMlvBHZwj1XqFyCbOGABzkxl9sW4+uk= | |||
- secure: UYglcVukRlhS09V7MXwMQ5pU6gZZZ+bsaKnCSlPV+CQyh+ExabEYYmF7NBJirF/RDK30tbuI6mlSYagjX18PpLZ3390WI3WkKRbP0F1SfylHKlMh5MDcKNTsSwYMcQ0BX7teg7kpYmxbigP7jing8LPehP/QdQAhnkhpdXe1P/o= | |||
mono: 5.2.0 | |||
mono: latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's hold off on this. I've got things working on 5.2.0 again with the latest Mono package.
build.json
Outdated
@@ -6,12 +6,12 @@ | |||
"1.1.2" | |||
], | |||
"LegacyDotNetVersion": "1.0.0-preview2-1-003177", | |||
"RequiredMonoVersion": "5.2.0.196", | |||
"RequiredMonoVersion": "5.8.0.129", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not change this. The latest Mono package works fine when 5.2.0.196 is installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, that's great. so let's merge #1137 and then I'll rebase this and get rid of these changes
merged. Feel free to update this PR. |
a4cb437
to
6634003
Compare
Done - should be good to go as soon as the build completes |
...as soon as the feed stops returning 503 🙂 |
Yeah. 😞 I've been restarting the builds multiple times. It seems like myget is intermittently failing. |
Finally green! |
🚀🚀 |
The stable packages are out.