Skip to content

Conversation

@vaeksare
Copy link
Member

@vaeksare vaeksare commented Nov 29, 2018

Adds a new build script flag that allows specifying that certain RID agnostic files inside redist should not be built/executed. This is used to prevent the DNNImageFeaturizer models from being copied on each leg causing possible build failures. Instead, the copying only happens on Windows x64. Fixes #1775

DependsOnTargets="DownloadDnnModelFiles">
<Message Importance="High" Text="Copying external model files... @(ModelFileCopy->'%(TargetPath)')" />
<Copy SourceFiles="@(ModelFileCopy)"
Condition="'$(SkipRIDAgnosticAssets)' == ''"
Copy link
Member

@eerhardt eerhardt Nov 29, 2018

Choose a reason for hiding this comment

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

Condition checking in MSBuild is a little different. Typically "boolean" properties will have 3 values: empty, true, and false. So generally, you pick empty (the default) to be considered the same as either 'true' or 'false'. In our case here, the default for SkipRIDAgnosticAssets should be false. So instead it is best to write this condition as:

Condition="'$(SkipRIDAgnosticAssets)' != 'true'"

That way someone can explicitly set /p:SkipRIDAgnosticAssets=false, and it still works correctly. #Resolved

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

Looks good - with just one minor condition-writing suggestion.

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

Looks great! thanks.

@eerhardt
Copy link
Member

Let's get this in, since 2 more builds just failed because of this.

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

@eerhardt eerhardt merged commit 83e33db into dotnet:master Nov 29, 2018
@ghost ghost locked as resolved and limited conversation to collaborators Mar 26, 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