Add ignore scopes to props file for local builds

[weshaggard]

Today the ignore scopes are only passed in via our build definitions
but that make it hard to keep updated as well as makes it difficult to
reproduce the same builds locally. To help with that we are committing
the list to a props file in the repo.

cc @shahabhijeet @kurtzeborn

[shahabhijeet]

LocationBasedService is fixed, so we no longer have to ignore it.
Any reason we are skipping KeyVault Tests? I thought none of the KeyVault tests need to be ignored?

[weshaggard]

I'm happy to adjust the set but for this PR I was planning to keep the list exactly the same as the existing build definition then we can tweak it. Let me know and I'll be happy to remove LocationBasedService and KeyVault.

[shahabhijeet]

Sounds good.

[shahabhijeet]

@weshaggard can we move this import as the very last in this file.
I usually keep all imports either in the beginning or towards the end, unless we have a very specific reason to be in the middle?
In this case it is better to import it at the end.

[weshaggard]

Unfortunately this has to be at the end of the project with the current way it is setup in the build tools. See https://github.com/Azure/azure-sdk-for-net/blob/BuildToolsForSdk/tools/BuildAssets/targets/common.Build.props#L17, that blindly sets DefaultPathTokenToIgnore so if we want to move this then we would need to submit a fix for that as well.

[weshaggard]

I'll submit a change in the LTSBuildTools branch to enable this behavior.

[shahabhijeet]

@weshaggard Ah I see what you mean.
But correct me if I am wrong @weshaggard, even the route you have opted for will get overwritten by the behavior for $(DefaultPathTokenToIgnore) in common.Build.props (unless I fail to understand this change)
But here is something to consider and let me know what you think, We can introduce a new ItemGroup for each inidividal project to be ignored, but then initialize $(IgnorePathTokens) with the all the paths. This way $(DefaultPathTokenToIgnore) remains local to common.Build.props and never get's leaked outside common.Build.props.
Another alternative is to simply initialize $(IgnorePathTokens) with the paths that we want to ignore and no need to create a new ItemGroup.
Either approaches works for me.
Thoughts?

[weshaggard]

But correct me if I am wrong @weshaggard, even the route you have opted for will get overwritten by the behavior for $(DefaultPathTokenToIgnore) in common.Build.props (unless I fail to understand this change)

No it wouldn't because it happens after the property is set in common.Build.props.

Another alternative is to simply initialize $(IgnorePathTokens) with the paths that we want to ignore and no need to create a new ItemGroup.

We cannot reuse IgnorePathTokens property and keep that working if you pass it as a command line parameter. Properties passed as command line parameters to msbuild cannot be mutated in props/targets files. So we would need to create another item group if we want both the command line parameter and the props to work together.

I think #5075 will enable the scenario we want and still allow us to set the properties before the props file import.

[shahabhijeet]

@weshaggard #5057 looks great, it will allow us to move the imports at the end of the file.
Correct me if I am wrong, properties passed at the cmdline are immutable in the global scope prior to completion of target execution. You can always override cmdline properties from within the target.
Let's just move the imports at the end and let's try this change out.

[weshaggard]

Correct me if I am wrong, properties passed at the cmdline are immutable in the global scope prior to completion of target execution. You can always override cmdline properties from within the target.

No I don't believe you can even change them inside a target they are fixed. The only hacky way to change a property that was past at the command line is to use the TreatAsLocalProperty (see note in https://docs.microsoft.com/en-us/visualstudio/msbuild/how-to-build-the-same-source-files-with-different-options?view=vs-2017), which I don't suggest we do in this case and instead just use a different property name like we are doing.

[shahabhijeet]

So here is why I said it could be mutated from within the target

<Project>
  <PropertyGroup>
    <Prop1>Global-Init</Prop1>
  </PropertyGroup>

  <Target Name="First" DependsOnTargets="F1;F2">
    <Message Text="First-Prop1 ===== $(Prop1)"/>
  </Target>

  <Target Name="F1">
    <Message Text="F1-Prop1 ===== $(Prop1)"/>
  </Target>

  <Target Name="F2">
    <PropertyGroup>
      <Prop1>F2-Init</Prop1>
    </PropertyGroup>
    <Message Text="F2-Prop1 ===== $(Prop1)"/>
  </Target>

  <Target Name="Second">
    <Message Text="Second-Prop1 ===== $(Prop1)"/>
  </Target>
</Project>

Here are two scenarios I tried out
1.
msbuild .\TestScope.props /t:"First;Second"
F1:
F1-Prop1 ===== Global-Init
F2:
F2-Prop1 ===== F2-Init
First:
First-Prop1 ===== F2-Init
Second:
Second-Prop1 ===== F2-Init

msbuild .\TestScope.props /t:"First;Second" /p:Prop1=cmdLineInit
F1:
F1-Prop1 ===== cmdLineInit
F2:
F2-Prop1 ===== F2-Init
First:
First-Prop1 ===== F2-Init
Second:
Second-Prop1 ===== F2-Init

[weshaggard]

Hmmm I guess I never tried that but it does appear you can do it mutate in a target. Thanks for sharing.

However I still like the idea of using a different property as modifying it in a target potentially leaves any other static properties with the old value if they depend on it.

[shahabhijeet]

Sounds good.

[shahabhijeet]

@weshaggard Ah I see what you mean.
But correct me if I am wrong @weshaggard, even the route you have opted for will get overwritten by the behavior for $(DefaultPathTokenToIgnore) in common.Build.props (unless I fail to understand this change)
But here is something to consider and let me know what you think, We can introduce a new ItemGroup for each inidividal project to be ignored, but then initialize $(IgnorePathTokens) with the all the paths. This way $(DefaultPathTokenToIgnore) remains local to common.Build.props and never get's leaked outside common.Build.props.
Another alternative is to simply initialize $(IgnorePathTokens) with the paths that we want to ignore and no need to create a new ItemGroup.
Either approaches works for me.
Thoughts?

[shahabhijeet]

wont' this be redundant as we already have this
https://github.com/Azure/azure-sdk-for-net/pull/5075/files#diff-4eb2e39e9cee14f48932ee34ff35ad3dR17

[weshaggard]

Yes this will be redundant now.

[shahabhijeet]

Once the imports are moved to the end, this PR is ready to be merged.