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

Move the default location for ref assemblies to obj (from bin) #6543

Closed
rainersigwald opened this issue Jun 9, 2021 · 8 comments · Fixed by #6560, #6695 or #7075
Closed

Move the default location for ref assemblies to obj (from bin) #6543

rainersigwald opened this issue Jun 9, 2021 · 8 comments · Fixed by #6560, #6695 or #7075

Comments

@rainersigwald
Copy link
Member

Today, ref assemblies are copied into the bin/ref folder by default. However, this is a sometimes confusing and unexpected output that is primarily used for internal build-performance reasons.

For Visual Studio 2022 we'd like to move this to a folder under obj.

The code change is to update this:

<TargetRefPath Condition=" '$(TargetRefPath)' == '' and '$(ProduceReferenceAssembly)' == 'true' ">$([MSBuild]::NormalizePath($(TargetDir), 'ref', $(TargetFileName)))</TargetRefPath>

The compat impact is less clear; some customers may have projects that expect the presence in ref. We could provide an opt-out (and/or put this under a changewave) but we'd like to hear from anyone who objects to this plan.

@rainersigwald
Copy link
Member Author

Microsoft-internal link to discussion where we were unable to remember any specific reason that it should be in bin. .

@jaredpar
Copy link
Member

jaredpar commented Jun 9, 2021

We could provide an opt-out (and/or put this under a changewave) but we'd like to hear from anyone who objects to this plan.

Can't any customer that prefers the current location just override $(TargetRefPath) to match the current pre-Dev17 logic?

@rainersigwald
Copy link
Member Author

Can't any customer that prefers the current location just override $(TargetRefPath) to match the current pre-Dev17 logic?

Yes, but they'd have to discover it. I think that's likely good enough personally but I've been wrong before . . .

@Nirmal4G
Copy link
Contributor

Nirmal4G commented Jun 15, 2021

I'm not okay with obj since, reference assembly is also the final output of a build. Much as it would be confusing for some people that we would have it in bin it would also be more confusing if it's under obj.

This is also why I proposed a root build folder, where we can keep Assets which doesn't come under bin or obj. This way, we could have build\ref along side build\bin, build\obj and build\ext (NuGet, Paket and other MSBuild extensions).

@rainersigwald
Copy link
Member Author

I'm not okay with obj since, reference assembly is also the final output of a build.

Is it? It is generally an internal implementation detail of intermediate assemblies in the build. It's not generally shipped to customers, for example. In the cases where it is, you can opt to specify that it go in your own bin folder.

@Nirmal4G
Copy link
Contributor

True but I think of them as a stripped down version of the main Assembly, much like .NET Framework reference assemblies!

I'm not okay with either way, bin or obj but having it in bin\ref for final output and obj\ref for intermediate output made sense. This proposal takes away that intuition.

I do get that some do copy/move all assemblies by wildcard and that might be confusing but producing reference assemblies is not a novice feature. So, in that perspective, we can keep it as it is or we could introduce build/publish folders early on as an opt-in feature.

ladipro pushed a commit that referenced this issue Jul 20, 2021
Fixes #6543

### Context

The location of this file is mostly irrelevant, and users find it
confusing when it's in the output folder.

### Changes Made

Changed the compiler's obj output to a folder named `refint` and changed the
"final" output ref assembly to live in `obj/ref` (modulo other variables).

Did this under a double opt-out: it's in the 17.0 changewave, and there's a new property `$(ProduceReferenceAssemblyInBin)`.

### Notes

Naming on . . . everything is up for debate.

Co-authored-by: Nirmal Guru <Nirmal4G@gmail.com>
ladipro added a commit that referenced this issue Jul 21, 2021
Fixes #6543 

### Context

#6560 made `TargetRefPath` relative by mistake, which broke build in Visual Studio.

### Changes Made

Use `MSBuildProjectDirectory` to properly root the path.

### Testing

Verified that VS build works with this fix.

### Notes

```
0:010> !pe
Exception object: 0000020fa4de3a90
Exception type:   System.IO.DirectoryNotFound
ExceptionMessage:          Could not find a part of the path 'C:\WINDOWS\system32\obj\Debug\net6.0\ref\HelloWorld.dll'.

0:010> !clrstack
OS Thread Id: 0x95c (10)        Child SP               IP Call Site
000000bc6023cf88 00007ffb12634ed9 [HelperMethodFrame: 000000bc6023cf88]
000000bc6023d070 00007ffb037dba49 System.IO.__Error.WinIOError(Int32, System.String)
000000bc6023d0c0 00007ffb0408d7e6 System.IO.File.InternalCopy(System.String, System.String, Boolean, Boolean)
000000bc6023d150 00007ffaa7303d70 Microsoft.CodeAnalysis.BuildTasks.CopyRefAssembly.Copy()
000000bc6023d1b0 00007ffaa7303bad Microsoft.CodeAnalysis.BuildTasks.CopyRefAssembly.Execute()
```
@rainersigwald
Copy link
Member Author

This was reverted so reopening. Blocked by IDE support (dotnet/project-system#7444 and dotnet/roslyn#55244).

@rainersigwald rainersigwald reopened this Aug 2, 2021
@marcpopMSFT marcpopMSFT modified the milestones: 17.0, MSBuild 17.1 Sep 10, 2021
rainersigwald added a commit that referenced this issue Nov 30, 2021
Reverts #6718

Fixes #6543. The blocking issues in other layers have been resolved.
rainersigwald added a commit to dotnet/sdk that referenced this issue Dec 1, 2021
dotnet-maestro bot added a commit to dotnet/sdk that referenced this issue Dec 7, 2021
[release/6.0.2xx] Update dependencies from dotnet/msbuild


 - Update VS package version

 - Stop assuming ref assemblies are in output folder

They are moved--see dotnet/msbuild#6543

 - fixup! Stop assuming ref assemblies are in output folder

 - fixup! Stop assuming ref assemblies are in output folder

 - fixup! fixup! Stop assuming ref assemblies are in output folder

 - Only test ref outputs on newer MSBuild

Specifically these will fail due to an extra file in bin before the VS MSBuild has dotnet/msbuild#7075.

 - fixup! Only test ref outputs on newer MSBuild
degory added a commit to degory/ghul-targets that referenced this issue Feb 27, 2022
- Fix reference assembly output path (see dotnet/msbuild#6543)

bumps #minor version
degory added a commit to degory/ghul-targets that referenced this issue Feb 27, 2022
- Fix reference assembly output path (see dotnet/msbuild#6543)
- Remove 'true' from nuget push --no-symbols parameter in CI workflow (see NuGet/Home#11601)
- Use standard .NET 6 SDK container image in CI workflow

bumps #minor version
@AR-May AR-May added the triaged label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment