Skip to content
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

ApiCompat no longer passes IntermediateOutputPath #5361

Open
2 tasks
sbomer opened this issue Apr 27, 2020 · 7 comments
Open
2 tasks

ApiCompat no longer passes IntermediateOutputPath #5361

sbomer opened this issue Apr 27, 2020 · 7 comments
Labels
area-Infrastructure-libraries Area maintained by .NET libraries team: APICompat, AsmDiff, GenAPI, GenFacades, PkgProj, etc help wanted

Comments

@sbomer
Copy link
Member

sbomer commented Apr 27, 2020

  • This issue is blocking arcade update in mono/linker
  • This issue is causing unreasonable pain

An arcade update in mono/linker is failing on ApiCompat.

It looks like ApiCompat was changed to no longer pass IntermediateOutputPath: d6dd7a0#diff-3c3f2c92eeec9b69dac2deccfac46603L72, so it isn't resolving the implementation assembly.

@dougbu is this expected? If so, what's the supported way to tell ApiCompat where to find the IntermediateAssembly?

sbomer added a commit to dotnet/linker that referenced this issue Apr 27, 2020
@dougbu
Copy link
Member

dougbu commented Apr 27, 2020

Yes, this is expected. We were not aware of scenarios in which $(IntermediateOutputPath) mattered. What is the case @ericstj and I missed?

/fyi the previous conversation starts with #4585 (comment)

@sbomer
Copy link
Member Author

sbomer commented Apr 27, 2020

The scenario is an implementation csproj that has a ProjectReference to its contract project (with ReferenceOutputAssembly false) for comparison. In this case ReferencePath doesn't contain the intermediate assembly.

Thanks for the pointer - I gave the discussion a read, but don't think I have all the context necessary to understand the details. I'm happy to change how we invoke it - that was just how I saw others (dotnet/runtime IIRC) doing it at the time.

@dougbu
Copy link
Member

dougbu commented Apr 27, 2020

@ericstj how do you prefer we handle this?

dotnet-maestro bot added a commit to dotnet/linker that referenced this issue Apr 27, 2020
* Update dependencies from https://github.com/dotnet/arcade build 20200421.14

- Microsoft.DotNet.ApiCompat: 5.0.0-beta.20201.2 -> 5.0.0-beta.20221.14
- Microsoft.DotNet.Arcade.Sdk: 5.0.0-beta.20201.2 -> 5.0.0-beta.20221.14

* Add dotnet-tools nuget feed

* Work around ApiCompat change

dotnet/arcade#5361

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Alexander Köplinger <[email protected]>
Co-authored-by: Sven Boemer <[email protected]>
@ericstj
Copy link
Member

ericstj commented Apr 28, 2020

@dougbu my suggestions was based on --contract-depends, not --impl-dirs. In the case of --contract-depends IntermediateOutputPath is not needed since the actual contract is passed in. In the case of --impl-dirs it is required. I think you should undo your change to --impl-dirs

@dougbu
Copy link
Member

dougbu commented Apr 28, 2020

@ericstj I'm fine reverting that change, restoring $(IntermediateOutputPath) in the forward comparison's --impl-dirs. However, that option is exactly the same as --contract-depends in the reversed comparison. I'll fix things up in both (tomorrow).

dougbu added a commit that referenced this issue Apr 29, 2020
- #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
@dougbu
Copy link
Member

dougbu commented Apr 29, 2020

Opened #5373.

@sbomer do you have a way to test this out locally before we get this in and so on? For example, changing files in your packages folder with the two-word update should work.

@sbomer
Copy link
Member Author

sbomer commented Apr 29, 2020

@dougbu thanks! Yes, I've confirmed that the fix works locally. I also confirmed that just the ApiCompatArgs part of the fix is enough for our scenario.

dougbu added a commit that referenced this issue Apr 29, 2020
- #5361
- pass property value in `--impl-dirs` argument in `ValidateApiCompatForSrc` target
- reverts small part of #4585
  - specifically, restore one line changed in 24d6981
@ericstj ericstj added the area-Infrastructure-libraries Area maintained by .NET libraries team: APICompat, AsmDiff, GenAPI, GenFacades, PkgProj, etc label Mar 7, 2022
tkapin pushed a commit to tkapin/runtime that referenced this issue Jan 31, 2023
* Update dependencies from https://github.com/dotnet/arcade build 20200421.14

- Microsoft.DotNet.ApiCompat: 5.0.0-beta.20201.2 -> 5.0.0-beta.20221.14
- Microsoft.DotNet.Arcade.Sdk: 5.0.0-beta.20201.2 -> 5.0.0-beta.20221.14

* Add dotnet-tools nuget feed

* Work around ApiCompat change

dotnet/arcade#5361

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Alexander Köplinger <[email protected]>
Co-authored-by: Sven Boemer <[email protected]>

Commit migrated from dotnet/linker@4d6d8ad
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Infrastructure-libraries Area maintained by .NET libraries team: APICompat, AsmDiff, GenAPI, GenFacades, PkgProj, etc help wanted
Projects
None yet
Development

No branches or pull requests

4 participants