-
Notifications
You must be signed in to change notification settings - Fork 789
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
Fix build #1967
Fix build #1967
Conversation
Good catch, I missed this one in #1903. |
With this fix applied manually I'm getting this after @brettfo's recent pr:
Any pointers? |
Yes, I've got the very same error on my branch after rebasing on current master. |
Since the NRE is coming from the SDK I'm going to defer to @tinaschrepfer here. |
Why it is so important to make all these breaking stuff right in master?? Do it in a branch, then merge it into master when it's all green... |
ouch. is the master build broken again? |
I don't know why this PR is green. I merged it into my branch and the error is still here. |
@vasily-kirichenko Separate errors. The 'EnableExtension' part is probably not run on CI at all. |
@rojepp so the guys don't normally run |
@KevinRansom @dsyme can someone please still merge this? red on master is really break our flow. |
@forki it does not help with VisualFSharp.sln because it fails, see the error above. This means |
I know but with the CI at least
|
DeployExtension is false on CI, it's this step that's failing:
|
This is the callstack for the failure in the DeployExtension task:
|
When installing the VSIX by hand using
I get Here's the log:
|
I'm wondering if this means the master branch will now only install into VS2017 RC2.... (Which is not publicly available yet, nor do I know how to get an early version inside MS) |
@vasily-kirichenko @rojepp - if you are using VS2017 RC, please base your work on branch vs2017-rc. Do not pull master, but still send the PR to master. For example just do
but don't do
until you install RC2 when it comes out. In more detail, this commit is the head of vs2017-rc and changes to the previous VSSDK and if you work from that commit you can deploy extensions into RC and debug them using F5 as before. This commit is in master too but I immediately reverted it so master is exactly as it was before. This means your PRs will integrate smoothly |
So master is still broken for potential new contributors? It should be the other way around imho. The currently breaking changes should be in RC2 branch (which even seem to be named correctly for it! :) ) |
Sorry, I meant RC. I've adjust my comment above
Yes, of course. However it's up to the Visual F# Tools team though and they are currently using master as the branch where they are getting these tools to work with RC2 (hopefully ready for release soon, or perhaps as part of RC3, I don't know) and it's like 5am in the morning there. We just have to live with it for today. What I've done is a way for you to continue to work on PRs while things land. We've faced the same problem many times before (e.g. repo was only usable with VS2017, but VS2017 was only in Preview 1 or something and no one had it installed). The problems are an inherent part of shipping in Visual Studio but are becoming much less frequent because of the extensive use of CI against publicly available SDKs. Please let me give an internal Microsoft perspective.
So from the internal Microsoft perspective the focus is on integrating with RC2 or RC3. It's not on keeping master live and running on the soon-to-be-thrown-away RC. I know that from the OSS perspective the number 1 priority is to keep master contributor-ready. But equally that can pile up huge integration pressure where the repo hasn't been properly used against latest VS releases (even internally), and by the time things get integrated it's too late to fix anything in VS. Anyway, in general we've been pretty good at making this repo more contributor-ready. Please just accept this sort of thing as a normal part of shipping as part of Visual Studio. |
tl;dr - I've fully reverted my breaking changes in Detailed VersionContinuing off of what @dsyme just said, I'm going to give out a little too much information, but my direct manager is out of the office until Monday, so maybe he'll forget by then :) There are two issues that worked in concert to create this storm: 1. Setup ChangesThere have been a lot of changes in VS recently around setup authoring (you've see this if you've installed VS "15" Preview 4 or later) and many of those changes were very late-breaking. E.g., the setup team would commonly send an email along the lines of "We've changed how 'X' works, so now each team needs to update their component by 'Y'" where 'Y' is two days from that email getting sent out. One of these changes was in how F# creates VSIX packages, and the change on the F# end was to update the 2. F# Infrastructure ChangesIn the past our F# infrastructure with regards to merging between public and internal-only branches and making the internal VS builds use newer F# bits was all manual and I ain't about that life. I updated some tools so that everything would happen automatically. Where that got usOn the first day that the tools I mentioned in (2) above fully ran, there was also one of the changes that I mentioned in (1) which meant that internal VS builds couldn't use F# which in turn meant that internal VS builds were still using the F# bits from RC, and let's be honest, those weren't exactly the most stable. That kicked off a fire where I was trying to make F# Why I did what I didCertain things I'm not allowed to say. Certain things like: we've been internally testing a build of VS that we wanted to release as RC2, but that version had the old F# bits from the first RC. I'm also not allowed to say that through some pleading we got some higher-ups to agree to push back the official RC2 build by a few days so that we could get newer F# bits in, so this got the clock ticking. Ideally I would have made these changes in another What will be done in the futureIn short, once the RC2 release calms down I'll be doing the work to create an alternate So again, sorry about that, but I think you'll like the outcome. Ninja edit: |
Thanks for the explanation @brettfo ! Good luck with getting the bits inserted :) |
We don't often get an insight into the shit-ton of work that goes into this on your end. |
Thanks for being so open and thanks for all the hard work @brettfo. Kudos! |
Thanks for explanation! |
We are with you in spirit, we are your brothers and sisters in this battle
Yes yes yes yes yes yea yes yes yes yes yes yes yes! |
FYI, VS 2017 RC2 has now been publicly released. PR to update |
Build was failing, I think this is the right fix @brettfo