Skip to content

Fix official build #626

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

Merged
merged 1 commit into from
Aug 1, 2018
Merged

Fix official build #626

merged 1 commit into from
Aug 1, 2018

Conversation

eerhardt
Copy link
Member

@eerhardt eerhardt commented Aug 1, 2018

Our official build is broken because we introduced a new dependency when building native code.

Previously, when we built native code, we didn't need to restore NuGet packages. But with #624 we now have a dependency from our native C++ code to the MklImports NuGet package. And our build fails if the package hasn't been restored.

The fix is to make BuildNative dependent on RestorePackages.

…nux. But building native now requires a Restore to happen first.
@eerhardt eerhardt requested review from codemzs and safern August 1, 2018 19:59
@codemzs
Copy link
Member

codemzs commented Aug 1, 2018

Thank you very much for the fix!

Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

LGTM.

I just have a question would: https://github.com/eerhardt/machinelearning/blob/46374da079f0d52cbe8b96134510dd61dd3a7506/build.proj#L33

Cause this target to be run twice?

@eerhardt
Copy link
Member Author

eerhardt commented Aug 1, 2018

Cause this target to be run twice?

No. MSBuild targets don't run twice.

https://docs.microsoft.com/en-us/visualstudio/msbuild/target-build-order

A target is never run twice during a build, even if a subsequent target in the build depends on it. Once a target has been run, its contribution to the build is complete.

@safern
Copy link
Member

safern commented Aug 1, 2018

No. MSBuild targets don't run twice.

Oh yeah, you're right, completely forgot about that. Thanks!

@eerhardt eerhardt merged commit 89dfc82 into dotnet:master Aug 1, 2018
@eerhardt eerhardt deleted the FixBuild branch August 1, 2018 20:52
@ghost ghost locked as resolved and limited conversation to collaborators Mar 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants