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

Generate F# assemblyinfo in new fsproj #3113

Closed
enricosada opened this issue May 25, 2017 · 66 comments · Fixed by #4430 or #7024
Closed

Generate F# assemblyinfo in new fsproj #3113

enricosada opened this issue May 25, 2017 · 66 comments · Fixed by #4430 or #7024
Labels
Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Milestone

Comments

@enricosada
Copy link
Contributor

new fsproj should generate assemblyinfo attributes from props.

ref dotnet/netcorecli-fsc#93

  • in dotnet/sdk is done in the language agnostic target Microsoft.NET.GenerateAssemblyInfo.targets#L93-L96 using the WriteCodeFragment task
  • atm the F# language is not implemented, so doesnt generate the file with attributes
  • the WriteCodeFragment is defined inside Microsoft/msbuild and contains a version with CodeDom and another for coreclr (just doing stringbuilder), and support both c# and vb

As a note i tried to add the F# in the same way as C#/VB, but atm PR was stopped for a design decision
dotnet/msbuild#2132 (comment) /cc @KevinRansom @cartermp

@KevinRansom
Copy link
Member

@enricosada

To do this for the SDK we can override in our targets file and sdk apps will work fine.

https://github.com/dotnet/sdk/blob/8204b6b1f3ff2f9717c5d66ab903795271de2e9c/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.NET.GenerateAssemblyInfo.targets#L51

But we should also send a PR to the dotnet sdk refactoring the current implementation a bit.
@nguerrera said he would welcome a PR that improved this :-)
The bit where the attributes are generated should be moved to a new target: GetAssemblyAttributes or some such name.
And then : CoreGenerateAssemblyInfo just handles the write.
We can override CoreGenerateAssemblyInfo to write the attributes with a build task added to the Microsoft.FSharp.Build library ... or something.
Does that make sense?
Enrico,

thanks for pushing on this, it's really great that you take the time to dive into all of the stuff.
Kevin

@enricosada
Copy link
Contributor Author

Does that make sense?

Yes, didnt know a refactoring effort was needed or ongoing, but is getting better. love to override only what's needed 👍

Let's wait a bit for new @dasMulli PR ( from dotnet/msbuild#2132 (comment) ) seems that refactoring is in progress already

There's already dotnet/sdk#1007 which splits the target into _CalculateAssemblyAttributes and CoreGenerateAssemblyInfo but it probably won't land in its current form (see PR discussion. I'll be sending out a PR today for an alternative implementation)

Meanwhile i'll prepare a task to write the file to be added in Microsoft.FSharp.Build because attrs need escape checks anyway.
Cannot be done with msbuild target alone afaik, sigh why msbuild property functions are so limited, when i need just a string -> string and some % transformation).

So as your said we'll need to just override CoreGenerateAssemblyInfo later, i'll PR soon, meanwhile copying the current CoreGenerateAssemblyInfo from sdk to check is everything working ok end2end

@KevinRansom
Copy link
Member

KevinRansom commented May 25, 2017 via email

@dsyme dsyme added the Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. label May 25, 2017
@enricosada
Copy link
Contributor Author

enricosada commented May 25, 2017

@KevinRansom yes @dasMulli is doing that with dotnet/sdk#1255

  1. _CalculateAssemblyAttributes which creates all AssemblyAttribute items.
  2. _CreateGeneratedAssemblyInfoInputsCacheFile creates an obj/tfm/projectname.AssemblyInfoInputs.cache file containing the hash of all AssemblyAttribute items. This filename is available as $(GeneratedAssemblyInfoInputsCacheFile). The target uses msbuild 15's WriteOnlyWhenDifferent="True" feature for WriteLinesToFile so the file doesn't change unnecessarily.
  3. CoreGenerateAssemblyInfo which generates the projectname.AssemblyInfo.cs file, using the cache file as input for incremental build.

The _CreateGeneratedAssemblyInfoInputsCacheFile is transparent for us

we will override just CoreGenerateAssemblyInfo to generate the file from AssemblyAttribute items.
i'll send a PR for that, you can assign this to me

@dasMulli
Copy link

<Target Name="CoreGenerateAssemblyInfo"
        DependsOnTargets="CreateGeneratedAssemblyInfoInputsCacheFile"
        Inputs="$(GeneratedAssemblyInfoInputsCacheFile)"
        Outputs="$(GeneratedAssemblyInfoFile)">

Should now do the trick, the $(GeneratedAssemblyInfoInputsCacheFile) input is only to trigger the target on incremental builds if any attribute changes but the file's contents can be ignored.

@cartermp cartermp added this to the VS 2017 Updates milestone Jun 3, 2017
@dsyme
Copy link
Contributor

dsyme commented Jun 19, 2017

@enricosada @KevinRansom Where are we with this - does it have to be done for the SDK work to count as feature complete? thx

@dsyme
Copy link
Contributor

dsyme commented Jun 21, 2017

@enricosada @KevinRansom ping... (I'm trying to clarify the status of all the "Urgency Now" bugs)

@enricosada
Copy link
Contributor Author

enricosada commented Jun 21, 2017

@dsyme i half finished it, i hope one or two days

@rmunn
Copy link

rmunn commented Aug 7, 2017

I recently found SuaveIO/suave#635 (assembly version should be 2.1.1.0 but is coming out as 0.0.0.0 when built with the dotnet core CLI tools -- when built with the .Net Framework tools or the Mono tools the assembly version is correct). At first I thought that was caused by an incorrect build step in the Suave project. But could that bug possibly be getting caused by this one? If the project is specifying the assembly version, but it's not making it into the DLL because of this bug, that would also explain the symptoms I'm seeing.

I'm completely lost when it comes to the .NET Core build system at the moment -- too many changes that I haven't been able to find documentation for -- so it's entirely possible that I'm barking up the wrong tree. But the fact that this bug is still open does make me wonder...

@dsyme
Copy link
Contributor

dsyme commented Aug 18, 2017

@enricosada @KevinRansom ping... (I'm trying to clarify the status of all the "Urgency Now" bugs)

@zvirja
Copy link

zvirja commented Aug 22, 2017

@dsyme @enricosada @KevinRansom Thank you guys for working on this one! happy to that I'm not the only one who have spotted it 😉

I'd say that this is very important (or even critical) issue as it breaks unification between C# and F#. Our solution contains both csproj and fsproj projects and currently I need to handle all fsproj files separately in the build scripts and patch their assembly info files manually.

Is there any chance that this issue will be resolved in the nearby future, so build scripts could be much simplified?

@KevinRansom
Copy link
Member

@zvirja yeah ... sorry about this ... it slipped through the cracks.

@KevinRansom
Copy link
Member

@enricosada, let me know if you want me to take care of this.
Also, If you want to push a WIP PR I can push it the last 100 yards.

@dsyme dsyme added Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code. and removed Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. labels Sep 1, 2017
@cmeeren
Copy link
Contributor

cmeeren commented Dec 16, 2017

In a .NET Core 2.0 console app, I've set the assembly version in the project properties, but when building with VS, the version is always missing (0.0.0.0). Is this caused by the issue described here?

I'm currently working around this by explicitly adding

[<assembly: System.Reflection.AssemblyVersionAttribute("2.0.0.0")>]

@zvirja
Copy link

zvirja commented Dec 17, 2017

@cmeeren Yep, that's because of this issue.

@dsyme @KevinRansom Do you know if it will be fixed somewhere soon? The VS 2017 update 5 did a great step towards the unification, while this issue still makes us to feel the difference between different SDKs.. 😕

@atlemann
Copy link

atlemann commented Jan 9, 2018

I have been struggling with this issue for days not getting the assembly info attributes in my compiled .dlls for F# projects. Will this be fixed? I'm running dotnet cli 2.1.3. Guess I'll have to do it the F# ProjectScaffold style for now.

@JonCanning
Copy link

I'm doing this in my build script for now

printf "\n[<System.Reflection.AssemblyVersion \"$ASSEMBLYVERSION\">] do()" >> ./src/Config.fs

@atlemann
Copy link

Hmm..seems that using the ProjectScaffold FAKE script's AssemblyInfo step does not work when building F# projects with dotnet CLI either.

@haf
Copy link

haf commented Jan 14, 2018

Registering my interest here, too, from #3549 (comment)

@edud69
Copy link

edud69 commented May 16, 2019

This seems to be fixed in VS 2019.

@lfr
Copy link

lfr commented May 20, 2019

Fixed as in a file is generated with new projects or fixed as in the contents of the Assemblyinfo file (version, icon, etc) are properly handled?

@atlemann
Copy link

VS2017 on the left, VS2019 on the right

image

@matthid
Copy link
Contributor

matthid commented May 20, 2019

@atlemann It was reported above that building within Visual Studio fixes the problem.
Issue is building with the SDK on the command line. Can you clarify that this is what you have tried?

The problem only happens in the dotnet-core runtime based compiler. Visual Studio runs the full-framework based one. (At least that was the state in VS2017)

@atlemann
Copy link

@matthid You're right.

.NET Core SDK: 2.2.204

Using dotnet build from CLI:

image

Building the same project using VS2019:

image

@KevinRansom
Copy link
Member

@ there is an issue with the coreclr compiler, that it generates the attributes fine, however, it doesn't embed them as a resource in the resource fork. This is a known limitation of the F# compiler for coreclr, it is a fair sized amount of work to make it work and so it will be addressed when priorities allow.

Kevin

@dasMulli
Copy link

dasMulli commented May 23, 2019

@atlemann can you try a 3.0 preview CLI (even when building for < 3.0)? The new build tasks in the CLI copy over resources from the dll file to the apphost executable when building on windows machines.

@nguerrera
Copy link

@dasMulli I believe the issue is that the dll won't have the resources in the first place when compiled with the F# compiler running on .NET Core.

@rojepp
Copy link
Contributor

rojepp commented Jun 20, 2019

@dasMulli I still see this issue in 3.0.100-preview6-012264 using dotnet publish/dotnet build.
Building from VS does work.

I also wanted to note that passing in version as a property does not work either: dotnet build /p:FileVersion=1.1.0

matthid added a commit to matthid/visualfsharp that referenced this issue Jun 20, 2019
@matthid
Copy link
Contributor

matthid commented Jun 21, 2019

I've sent a PR for this, so I hope we can finally get this sorted out. I understand this has low priority but it disturbs me every time I see it and makes it harder for people - especially for newcomers - to figure out version related issues... I have seen this several times

@realvictorprm
Copy link
Contributor

So the FileVersion thingy still needs to be fixed if I understand correctly?

@nguerrera
Copy link

I commented on the PR: #7024 (comment)

It seems there's a misunderstanding about work for managed resources becoming useful for this problem when they are actually separate.

@cartermp
Copy link
Contributor

This might be affecting #7220

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Projects
None yet