Skip to content

Pin VS BuildTools to 16.5.5 in order to avoid broken VC++ download.#462

Merged
brawner merged 2 commits intomasterfrom
brawner/pin-vs-version
Jun 3, 2020
Merged

Pin VS BuildTools to 16.5.5 in order to avoid broken VC++ download.#462
brawner merged 2 commits intomasterfrom
brawner/pin-vs-version

Conversation

@brawner
Copy link
Copy Markdown
Contributor

@brawner brawner commented Jun 3, 2020

The daily Dockerfile invalidation updated VS BuildTools to 16.6 which is apparently missing the visual studio package download for Visual C++ (as reported here). The result is that the python wheel installation of netifaces fails although without a C++ compiler the rest of the build isn't likely to pass if we worked around that.

By pinning back to the earlier Build Tools version we keep the old package which is still available and thus can continue building. We should be able to lift this pin when an updated Build Tools is available which includes the missing VC++ package.

Signed-off-by: Stephen Brawner brawner@gmail.com

Copy link
Copy Markdown
Member

@cottsay cottsay left a comment

Choose a reason for hiding this comment

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

LGTM once CI passes. Hopefully we'll have time to dig into what is going wrong later this week.

@dirk-thomas
Copy link
Copy Markdown
Member

Any comment to why this is done would be helpful in this ticket.

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner brawner force-pushed the brawner/pin-vs-version branch from 8988418 to d6b3816 Compare June 3, 2020 02:10
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Copy link
Copy Markdown
Contributor Author

brawner commented Jun 3, 2020

Added a comment to the dockerfiles. And pinned to 16.5.5 instead. CI build underway

Build Status

I have to go walk my dog, so if this passes and people want to merge, or modify the PR and merge please feel free.

@nuclearsandwich nuclearsandwich changed the title Pin VS BuildTools to 16.6 Pin VS BuildTools to 16.5.5 in order to avoid broken VC++ download. Jun 3, 2020
@nuclearsandwich
Copy link
Copy Markdown
Member

Any comment to why this is done would be helpful in this ticket.

I re-wrote the PR body and title with more context.

@brawner
Copy link
Copy Markdown
Contributor Author

brawner commented Jun 3, 2020 via email

@cottsay
Copy link
Copy Markdown
Member

cottsay commented Jun 3, 2020

I accidentally set ci_branch_to_test instead of ci_scripts_branch

Build Status

@brawner
Copy link
Copy Markdown
Contributor Author

brawner commented Jun 3, 2020 via email

@brawner
Copy link
Copy Markdown
Contributor Author

brawner commented Jun 3, 2020

Ok passing build. Merging

@brawner brawner merged commit 7cda3fd into master Jun 3, 2020
@delete-merged-branch delete-merged-branch bot deleted the brawner/pin-vs-version branch June 3, 2020 03:14
@dirk-thomas
Copy link
Copy Markdown
Member

@brawner Please create a follow up ticket to unpin.

@dirk-thomas
Copy link
Copy Markdown
Member

@brawner Did you run a full CI build with this change? Could these new warnings be related to this change: https://ci.ros2.org/job/ci_windows/10912/msbuild/new/

@cottsay
Copy link
Copy Markdown
Member

cottsay commented Jun 3, 2020

Could these new warnings be related to this change: https://ci.ros2.org/job/ci_windows/10912/msbuild/new/

Maybe the updated version of VS has something to do with why they're now being reported on the buildfarm, but those warnings are coming from ros2/rclcpp#1123. Seems like they should have been present all along, so it seems to me that this is an improvement.

@brawner
Copy link
Copy Markdown
Contributor Author

brawner commented Jun 3, 2020

Did you run a full CI build with this change?

No, just up to rcpputils. It's odd that reverting to an older version of VS buildtools would have brought up more warnings. Hopefully there isn't a bloodbath tonight...

@cottsay
Copy link
Copy Markdown
Member

cottsay commented Jun 3, 2020

Hopefully there isn't a bloodbath tonight...

I'd certainly rather we see some existing problems come to light than not get Windows builds at all :P

@brawner
Copy link
Copy Markdown
Contributor Author

brawner commented Jun 3, 2020

Followup issue #463

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.

4 participants