Skip to content

Commit

Permalink
Restore use of $(IntermediateOutputPath) in ApiCompat commands
Browse files Browse the repository at this point in the history 
- #5361
- pass property value in `--impl-dirs` argument in `ValidateApiCompatForSrc` target
- pass property value in `----contract-depends` argument in `RunMatchingRefApiCompat` target
- reverts small part of #4585
  - specifically, 24d6981
  • Loading branch information
@dougbu
dougbu committed Apr 29, 2020
1 parent 5269f17 commit e6e3669
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/Microsoft.DotNet.ApiCompat/build/Microsoft.DotNet.ApiCompat.targets
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
<ApiCompatArgs Condition="'$(ApiCompatExcludeAttributeList)' != ''">$(ApiCompatArgs) --exclude-attributes "$(ApiCompatExcludeAttributeList)"</ApiCompatArgs>
<ApiCompatArgs Condition="'$(ApiCompatEnforceOptionalRules)' == 'true'">$(ApiCompatArgs) --enforce-optional-rules</ApiCompatArgs>
<ApiCompatArgs Condition="'$(BaselineAllAPICompatError)' != 'true' and Exists('$(ApiCompatBaseline)')">$(ApiCompatArgs) --baseline "$(ApiCompatBaseline)"</ApiCompatArgs>
<ApiCompatArgs>$(ApiCompatArgs) --impl-dirs "@(_DependencyDirectories, ','),"</ApiCompatArgs>
<ApiCompatArgs>$(ApiCompatArgs) --impl-dirs "$(IntermediateOutputPath),@(_DependencyDirectories, ','),"</ApiCompatArgs>
<ApiCompatArgs Condition=" '$(AdditionalApiCompatOptions)' != '' ">$(ApiCompatArgs) $(AdditionalApiCompatOptions)</ApiCompatArgs>
<ApiCompatBaselineAll Condition="'$(BaselineAllAPICompatError)' == 'true'">&gt; "$(ApiCompatBaseline)"</ApiCompatBaselineAll>
<ApiCompatExitCode>0</ApiCompatExitCode>
Expand Down Expand Up @@ -123,7 +123,7 @@

<PropertyGroup>
<MatchingRefApiCompatArgs>$(MatchingRefApiCompatArgs) "$(ImplementationAssemblyAsContract)"</MatchingRefApiCompatArgs>
<MatchingRefApiCompatArgs>$(MatchingRefApiCompatArgs) --contract-depends "@(_ContractDependencyDirectories, ','),"</MatchingRefApiCompatArgs>
<MatchingRefApiCompatArgs>$(MatchingRefApiCompatArgs) --contract-depends "$(IntermediateOutputPath),@(_ContractDependencyDirectories, ','),"</MatchingRefApiCompatArgs>

This comment has been minimized.

Sign in to view

Copy link
@ericstj

ericstj Apr 29, 2020

Member

You do not need this here, since you don't expect anything other than$(ImplementationAssemblyAsContract) in $(IntermediateOutputPath)

This comment has been minimized.

Sign in to view

Copy link
@dougbu

dougbu Apr 29, 2020

Author Member

Not sure I'm getting your point @ericstj This is in the RunMatchingRefApiCompat target which does exactly the same calculations for @(_ContractDependencyDirectories) as ValidateApiCompatForSrc does for @(_DependencyDirectories). In other words, if $(IntermediateOutputPath) is needed in one place, it's needed in the other.

The main reason we didn't hear a complaint about RunMatchingRefApiCompat not working is because the target is used less often.

This comment has been minimized.

Sign in to view

Copy link
@ericstj

ericstj Apr 29, 2020

Member

No. We needed IntermediateOutputPath in --impl-dirs since that is the only way the built project's output assembly is considered as an implementation to satisfy the cotnract.

We don't need IntermediateOutputPath in --contract-depends since the built project's output is passed in as ImplementationAssemblyAsContract.

The piece you are missing here is that APICompat is not symmetric in how you specify inputs. For contract you pass in ContractAssemblies+DependencyDirectories. For implementation you pass in only Implementation directories. This is why the args are different in one target vs the other.

This comment has been minimized.

Sign in to view

Copy link
@dougbu

dougbu Apr 29, 2020

Author Member

Ah, yes, I missed that difference. And, I should have caught it during our long conversation in #4585 about the two targets and how they use $(IntermediateOutputPath).
<MatchingRefApiCompatArgs>$(MatchingRefApiCompatArgs) --left-operand implementation</MatchingRefApiCompatArgs>
<MatchingRefApiCompatArgs>$(MatchingRefApiCompatArgs) --right-operand reference</MatchingRefApiCompatArgs>
<MatchingRefApiCompatArgs Condition="'$(ApiCompatExcludeAttributeList)' != ''">$(MatchingRefApiCompatArgs) --exclude-attributes "$(ApiCompatExcludeAttributeList)"</MatchingRefApiCompatArgs>
Expand Down

0 comments on commit e6e3669

Please sign in to comment.