Skip to content

Conversation

@dougbu
Copy link
Contributor

@dougbu dougbu commented May 15, 2021

  • use empty $(MSBuildRestoreSessionId) to determine when contributing to dependency graph

  • remove extra direct references

    • should now be part of the dependency graph automatically

@dougbu
Copy link
Contributor Author

dougbu commented May 16, 2021

@dougbu dougbu force-pushed the dougbu/more.conditional.conditions branch from 1b7248d to ac705ab Compare May 17, 2021 00:06
dougbu added 5 commits May 16, 2021 22:23
- use empty `$(MSBuildRestoreSessionId)` to determine when contributing to dependency graph
- should now be part of the dependency graph automatically
- not a problem unless executing `restore` target
@dougbu dougbu force-pushed the dougbu/more.conditional.conditions branch from d09a27a to 82a5289 Compare May 17, 2021 05:23
@Pilchie Pilchie added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label May 17, 2021
@dougbu dougbu requested a review from a team May 19, 2021 22:59
@dougbu dougbu marked this pull request as ready for review May 19, 2021 23:00
@dougbu dougbu requested review from a team, Pilchie, Tratcher and halter73 as code owners May 19, 2021 23:00
@dougbu dougbu merged commit 0b0bed3 into main May 20, 2021
@dougbu dougbu deleted the dougbu/more.conditional.conditions branch May 20, 2021 18:50
@ghost ghost added this to the 6.0-preview6 milestone May 20, 2021
@dougbu
Copy link
Contributor Author

dougbu commented May 22, 2021

/backport to release/3.1

@github-actions
Copy link
Contributor

Started backporting to release/3.1: https://github.com/dotnet/aspnetcore/actions/runs/865552637

@ghost
Copy link

ghost commented May 22, 2021

Hi @github-actions[bot]. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@dougbu
Copy link
Contributor Author

dougbu commented May 22, 2021

/backport to release/5.0

@github-actions
Copy link
Contributor

Started backporting to release/5.0: https://github.com/dotnet/aspnetcore/actions/runs/865553258

@ghost
Copy link

ghost commented May 22, 2021

Hi @github-actions[bot]. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@github-actions
Copy link
Contributor

@dougbu an error occurred while backporting to release/3.1, please check the run log for details!

Cannot read property 'createComment' of undefined

@ghost
Copy link

ghost commented May 22, 2021

Hi @github-actions[bot]. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@github-actions
Copy link
Contributor

@dougbu an error occurred while backporting to release/5.0, please check the run log for details!

Cannot read property 'createComment' of undefined

@ghost
Copy link

ghost commented May 22, 2021

Hi @github-actions[bot]. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@github-actions
Copy link
Contributor

Started backporting to release/3.1: https://github.com/dotnet/aspnetcore/actions/runs/865552637

@ghost
Copy link

ghost commented May 22, 2021

Hi @github-actions[bot]. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@github-actions
Copy link
Contributor

@dougbu an error occurred while backporting to release/3.1, please check the run log for details!

Cannot read property 'createComment' of undefined

@ghost
Copy link

ghost commented May 22, 2021

Hi @github-actions[bot]. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@dougbu
Copy link
Contributor Author

dougbu commented May 22, 2021

@akoeplinger something strange is going on. The action is failing even after removing differences between this repo and dotnet/runtime in .github/workflows/backport.yml and eng/actions/backport/*, See #32777 then #32927.

I also verified https://octokit.github.io/rest.js/v18#issues-create-comment still matches what we're doing in this action.

Any ideas why either the issues property is undefined or GitHub's JS parser is interpreting createComment(...) as a property reference❔

@dougbu
Copy link
Contributor Author

dougbu commented May 22, 2021

@akoeplinger I figured it out. Need to use the rest object in one more place. See #32930. I'll put up a similar dotnet/runtime PR on Monday.

The other part of the issue here was conflicts when the action attempted to apply the patch. Anyone know if git am --3way would help create a PR containing the conflicts❔

@dougbu
Copy link
Contributor Author

dougbu commented May 22, 2021

Anyone know if git am --3way would help create a PR containing the conflicts❔

Oops, index.js already uses --3way. Probably need more complicated git hacks to create a PR despite the conflicts.

@dougbu
Copy link
Contributor Author

dougbu commented May 24, 2021

Let's try this again…

@dougbu
Copy link
Contributor Author

dougbu commented May 24, 2021

/backport to release/5.0

@github-actions
Copy link
Contributor

Started backporting to release/5.0: https://github.com/dotnet/aspnetcore/actions/runs/872340045

@ghost
Copy link

ghost commented May 24, 2021

Hi @github-actions[bot]. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@github-actions
Copy link
Contributor

@dougbu backporting to release/5.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Expose all references when not restoring - use empty `$(MSBuildRestoreSessionId)` to determine when contributing to dependency graph
Using index info to reconstruct a base tree...
M	src/DataProtection/DataProtection/src/Microsoft.AspNetCore.DataProtection.csproj
M	src/Identity/Extensions.Core/src/Microsoft.Extensions.Identity.Core.csproj
M	src/Logging.AzureAppServices/src/Microsoft.Extensions.Logging.AzureAppServices.csproj
M	src/Servers/Connections.Abstractions/src/Microsoft.AspNetCore.Connections.Abstractions.csproj
M	src/SignalR/clients/csharp/Http.Connections.Client/src/Microsoft.AspNetCore.Http.Connections.Client.csproj
M	src/SignalR/common/Http.Connections.Common/src/Microsoft.AspNetCore.Http.Connections.Common.csproj
M	src/SignalR/common/SignalR.Common/src/Microsoft.AspNetCore.SignalR.Common.csproj
M	src/Testing/src/Microsoft.AspNetCore.Testing.csproj
M	src/WebEncoders/src/Microsoft.Extensions.WebEncoders.csproj
Falling back to patching base and 3-way merge...
Auto-merging src/WebEncoders/src/Microsoft.Extensions.WebEncoders.csproj
Auto-merging src/Testing/src/Microsoft.AspNetCore.Testing.csproj
Auto-merging src/SignalR/common/SignalR.Common/src/Microsoft.AspNetCore.SignalR.Common.csproj
Auto-merging src/SignalR/common/Http.Connections.Common/src/Microsoft.AspNetCore.Http.Connections.Common.csproj
Auto-merging src/SignalR/clients/csharp/Http.Connections.Client/src/Microsoft.AspNetCore.Http.Connections.Client.csproj
Auto-merging src/Servers/Connections.Abstractions/src/Microsoft.AspNetCore.Connections.Abstractions.csproj
CONFLICT (content): Merge conflict in src/Servers/Connections.Abstractions/src/Microsoft.AspNetCore.Connections.Abstractions.csproj
Auto-merging src/Logging.AzureAppServices/src/Microsoft.Extensions.Logging.AzureAppServices.csproj
Auto-merging src/Identity/Extensions.Core/src/Microsoft.Extensions.Identity.Core.csproj
Auto-merging src/DataProtection/DataProtection/src/Microsoft.AspNetCore.DataProtection.csproj
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Expose all references when not restoring - use empty `$(MSBuildRestoreSessionId)` to determine when contributing to dependency graph
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@ghost
Copy link

ghost commented May 24, 2021

Hi @github-actions[bot]. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@Pilchie
Copy link
Member

Pilchie commented May 24, 2021

FYI - I think I just changed @msftbot to not comment when @github-actions[bot] comments on a closed PR.

dougbu added a commit that referenced this pull request Jun 1, 2021
- backport of 0b0bed3 (#32718)

* Expose all references when not restoring
  - use empty `$(MSBuildRestoreSessionId)` to determine when contributing to dependency graph
* Remove extra direct references
  - should now be part of the dependency graph automatically
* Avoid errors about non-shared Fx references
  - not a problem unless executing `restore` target
* Special case source builds
dougbu added a commit that referenced this pull request Jun 1, 2021
- backport of 0b0bed3 (#32718)

* Expose all references when not restoring
  - use empty `$(MSBuildRestoreSessionId)` to determine when contributing to dependency graph
* Remove extra direct references
  - should now be part of the dependency graph automatically
* Avoid errors about non-shared Fx references
  - not a problem unless executing `restore` target
* Special case source builds
dougbu added a commit that referenced this pull request Jun 4, 2021
* [release/5.0] Expose all references when not restoring
- backport of 0b0bed3 (#32718)

* Expose all references when not restoring
  - use empty `$(MSBuildRestoreSessionId)` to determine when contributing to dependency graph
* Remove extra direct references
  - should now be part of the dependency graph automatically
* Avoid errors about non-shared Fx references
  - not a problem unless executing `restore` target
* Special case source builds
dougbu added a commit that referenced this pull request Jul 14, 2021
- backport of 0b0bed3 (#32718)

* Expose all references when not restoring
  - use empty `$(MSBuildRestoreSessionId)` to determine when contributing to dependency graph
* Remove extra direct references
  - should now be part of the dependency graph automatically
* Avoid errors about non-shared Fx references
  - not a problem unless executing `restore` target
* Special case source builds
dougbu added a commit that referenced this pull request Aug 19, 2021
- backport of 0b0bed3 (#32718)

* Expose all references when not restoring
  - use empty `$(MSBuildRestoreSessionId)` to determine when contributing to dependency graph
* Remove extra direct references
  - should now be part of the dependency graph automatically
* Avoid errors about non-shared Fx references
  - not a problem unless executing `restore` target
* Special case source builds
dougbu added a commit that referenced this pull request Sep 8, 2021
* [release/3.1] Expose all references when not restoring
- backport of 0b0bed3 (#32718)

* Expose all references when not restoring
  - use empty `$(MSBuildRestoreSessionId)` to determine when contributing to dependency graph
* Remove extra direct references
  - should now be part of the dependency graph automatically
* Avoid errors about non-shared Fx references
  - not a problem unless executing `restore` target
* Special case source builds

* Add workaround for ref/ generation
  - use `$(CompileUsingReferenceAssemblies)` to avoid new references in generated projects

* Limit `_CheckForReferenceBoundaries` to Core TFM
  - back-ports a very small part of 5266918
  - avoid incorrect errors about extra references in shared framework
  - safe because every `$(IsAspNetCoreApp)` project targets (or multi-targets) `netcoreapp3.1`

* Add more `$(MSBuildRestoreSessionId)` checks
  - additional projects in this branch have TFM-specific references

* nit: Remove a few duplicate references
  - no need to mention Microsoft.AspNetCore.DataProtection twice

* Remove excess assemblies from shared Fx
  - brought in due to improved reference discovery
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants