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

Add FSharp.Build support for reference assemblies #13223

Open
vzarytovskii opened this issue Jun 1, 2022 · 14 comments
Open

Add FSharp.Build support for reference assemblies #13223

vzarytovskii opened this issue Jun 1, 2022 · 14 comments
Labels
Area-Build Everything related to building F# compiler, tooling and VS extension. Area-Compiler-CodeGen IlxGen, ilwrite and things at the backend Feature Improvement
Milestone

Comments

@vzarytovskii
Copy link
Member

Add or modify FSharp.Build tasks that will determine if projects need to be re-built or not depending on if the reference assembly output has changed.

See Roslyn's implementation of this: dotnet/roslyn@315c2e1/src/Compilers/Core/MSBuildTask/CopyRefAssembly.cs

@kerams
Copy link
Contributor

kerams commented Sep 2, 2022

Any chance of this making it into .NET 7?

@vzarytovskii
Copy link
Member Author

I believe it's already there, probably has issues though, since we didn't do a comprehensive testing.

@NinoFloris
Copy link
Contributor

Seems like RC2 will contain the enabling bits dotnet/sdk#26306

@vzarytovskii
Copy link
Member Author

Seems like RC2 will contain the enabling bits dotnet/sdk#26306

This is SDK support, FSharp.Build support went in earlier:
#13567
#13263

@NinoFloris
Copy link
Contributor

Right, I was looking for the sdk support because that's what I've been waiting for. @kerams and I are probably commenting on the wrong issue ;)

@charlesroddie
Copy link
Contributor

In VS 17.3.2 I find that reference assemblies seem to work between F# projects, but not F# -> C#. Is it that the feature isn't in released VS yet or is this a bug that I should post about separately?

@marcin-krystianc
Copy link
Contributor

In VS 17.3.2 I find that reference assemblies seem to work between F# projects, but not F# -> C#. Is it that the feature isn't in released VS yet or is this a bug that I should post about separately?

There was a bug that got fixed via #13567, I'm not sure in which version of VS it will be included though.

@safesparrow
Copy link
Contributor

Is this now complete?

@vzarytovskii
Copy link
Member Author

Is this now complete?

Not fully, I believe we don't make decisions to rebuild based on MVID yet.

@safesparrow
Copy link
Contributor

safesparrow commented Apr 22, 2023

Hmm, I'm confused.
This already works in multi-project scenarios, as in compilation is avoided when non-public parts of a dependency change.
Which must imply that the timestamps of the reference assembly files used in dependant projects do not get updated when non-public changes happen in dependencies.

I can also see that while obj\Debug\net7.0\refint\{name}.dll changes every time, obj\Debug\net7.0\ref\{name}.dll does not, and subsequent projects use the latter as a reference.

So to me it seems like nothing is missing.

The Roslyn implementation you linked to decides whether to copy a file (it only copies it if the MVID changed).
This part already works AFAICS.

@vzarytovskii
Copy link
Member Author

vzarytovskii commented Apr 22, 2023

Hmm, I'm confused.
This already works in multi-project scenarios, as in compilation is avoided when non-public parts of a dependency change.
Which must imply that the timestamps of the reference assembly files used in dependant projects do not get updated when non-public changes happen in dependencies.

I can also see that while obj\Debug\net7.0\refint\{name}.dll changes every time, obj\Debug\net7.0\ref\{name}.dll does not, and subsequent projects use the latter as a reference.

So to me it seems like nothing is missing.

The Roslyn implementation you linked to decides whether to copy a file (it only copies it if the MVID changed).
This part already works AFAICS.

They're not copied, but still emitted, we can be avoiding emitting those alltogether.

I.e. We can be passing existing MVID when generating reassemblies and if matches before ilxgen, we can just not do anything.

@safesparrow
Copy link
Contributor

safesparrow commented Apr 22, 2023

They're not copied, but still emitted, we can be avoiding emitting those alltogether.
I.e. We can be passing existing MVID when generating reassemblies and if matches before ilxgen, we can just not do anything.

Ok, that seems like an additional feature.
It's not implemented in C#, is it, given that both C# and F# use the refint intermediate directory for always emitting a ref assembly?

The ref assembly is then conditionally copied over to ref directory if it's changed - for C# it's done in the code you linked.

@vzarytovskii
Copy link
Member Author

They're not copied, but still emitted, we can be avoiding emitting those alltogether.
I.e. We can be passing existing MVID when generating reassemblies and if matches before ilxgen, we can just not do anything.

Ok, that seems like an additional feature.
It's not implemented in C#, is it, given that both C# and F# use the refint intermediate directory for always emitting a ref assembly?

The ref assembly is then conditionally copied over to ref directory if it's changed - for C# it's done in the code you linked.

Yeah, I wanted it to be an additional feature - to not even emit it if we don't have to.

@T-Gro
Copy link
Member

T-Gro commented Jun 28, 2023

Yeah, I wanted it to be an additional feature - to not even emit it if we don't have to.

Good news for us, there is a dedicated 'copy to final folder' step, which is clever and checks the mvid's of assemblies before/after.

  1. msbuild/src/Tasks/Microsoft.Common.CurrentVersion.targets at main · dotnet/msbuild · GitHub
    It is in the Common.*.targets, therefore (I hope!) this applies to all .net languages.

  2. This Task is written in Roslyn repo roslyn/src/Compilers/Core/MSBuildTask/CopyRefAssembly.cs at main · dotnet/roslyn · GitHub and works by opening the target folder (/bin), finding the "old" .dll, and reading it's mvid from the metadata section. Doing the same for the new one, and copying only if their MVID's are different => good.

    1. The MVID value itself is not passed using msbuild, i.e. the only info passed is the target folder location, and then the possibility to load it from a file is there.
  3. If we in F# do nothing, this will work for us as well. Ref assembly will be produced, but not copied over to /bin and therefore timestamp will not get updated.

  4. If we wanted to even stop producing the intermmediate reference assembly, we would need to do the same:

    1. Load the existing one by using msbuild properties about the target folder
    2. Load it's mvid by something similar to the roslyn/src/Compilers/Core/MSBuildTask/MvidReader.cs at main · dotnet/roslyn · GitHub ( => more code in compiler)
    3. Make sure we would not be breaking anything if not producing the expected intermmediate ref assembly (this might be the hardest part)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Build Everything related to building F# compiler, tooling and VS extension. Area-Compiler-CodeGen IlxGen, ilwrite and things at the backend Feature Improvement
Projects
Status: New
Development

No branches or pull requests

7 participants