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

Non-ResX resources get different resource IDs on Windows compared to C# #922

Closed
nosami opened this Issue Feb 1, 2016 · 17 comments

Comments

Projects
None yet
6 participants
@nosami
Member

nosami commented Feb 1, 2016

Folder names aren't respected when generating resource ids passed to the --resource parameter of fsc.exe

  • Create a new F# Console project
  • Add a folder called "AFolder"
  • Add a text file to AFolder called "AResourceFile.txt"
  • Set the build action on that text file to embedded resource
open System
open System.Reflection

[<EntryPoint>]
let main argv = 
    Assembly.GetExecutingAssembly().GetManifestResourceNames()
    |> Array.iter (Console.WriteLine) 
    0 // return an integer exit code

outputs

AResourceFile.txt

Performing the same steps in a C# project results in

csconsoleresource.AFolder.AResourceFile.txt

Note too that it isn't possible to override the resource id by using

<LogicalName>My.Custom.Identifier</LogicalName>

in the msbuild file.

See https://bugzilla.xamarin.com/show_bug.cgi?id=37971 and fsharp/fsharp#50 for further background.

@mexx

This comment has been minimized.

Show comment
Hide comment
@mexx

mexx Feb 1, 2016

Contributor

FYI when using the Resource build action, folders are correctly managed, albeit all packaged into the global resource, so some extra work is necessary to access them. You can look at an example of my GlobalResourceFileSystem for OWIN.

Contributor

mexx commented Feb 1, 2016

FYI when using the Resource build action, folders are correctly managed, albeit all packaged into the global resource, so some extra work is necessary to access them. You can look at an example of my GlobalResourceFileSystem for OWIN.

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Feb 7, 2016

Contributor

I seem to remember some subtle behaviour difference between the F# open edition and the Visual F# Tools here, due to some bug in Mono. It's possible that bug has now been fixed

@nosami could you check if this problem exists in Visual F# Tools, or just Mono F#, or both? thanks

Contributor

dsyme commented Feb 7, 2016

I seem to remember some subtle behaviour difference between the F# open edition and the Visual F# Tools here, due to some bug in Mono. It's possible that bug has now been fixed

@nosami could you check if this problem exists in Visual F# Tools, or just Mono F#, or both? thanks

@nosami

This comment has been minimized.

Show comment
Hide comment
@nosami

nosami Feb 7, 2016

Member

The two bugs are present in both. xbuild delegates the task of generating resource Ids to FSharp.Build.dll which appears to come from this repo.

Note that Xamarin Studio recently started using code from MSBuild to build the code, so there is very little difference in the build process any more (if any). At some time in the past, it used it's own mechanism (which does actually do the correct thing here)

I created a repro solution here

Member

nosami commented Feb 7, 2016

The two bugs are present in both. xbuild delegates the task of generating resource Ids to FSharp.Build.dll which appears to come from this repo.

Note that Xamarin Studio recently started using code from MSBuild to build the code, so there is very little difference in the build process any more (if any). At some time in the past, it used it's own mechanism (which does actually do the correct thing here)

I created a repro solution here

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Apr 8, 2016

Contributor

@nosami There's a long analysis of this here, and you're right that the two bugs described above do exist in the Windows implementation.

I can't for the life of me work out where LogicalName is consumed even by C# MSBuild on Windows, and why it would not be being applied for F#. If someone can point me in the right direction I'd be really grateful. This stuff is just impenetrable.

BTW am I right in thinking that the command msbuild is now available in the Mono linux packages?

Contributor

dsyme commented Apr 8, 2016

@nosami There's a long analysis of this here, and you're right that the two bugs described above do exist in the Windows implementation.

I can't for the life of me work out where LogicalName is consumed even by C# MSBuild on Windows, and why it would not be being applied for F#. If someone can point me in the right direction I'd be really grateful. This stuff is just impenetrable.

BTW am I right in thinking that the command msbuild is now available in the Mono linux packages?

@dsyme dsyme changed the title from Incorrect resource IDs generated by FSharp.Build to Non-ResX resources get different resource IDs on Windows compared to C# Apr 9, 2016

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Apr 9, 2016

Contributor

@nosami I've clarified that for Windows this only applies to non-ResX resources. For Linux, when using XBuild, the problem also applies to ResX resources.

Applying a fix for this on Windows would be a breaking change - do you see if there's a way to fix this without a breaking change?

Contributor

dsyme commented Apr 9, 2016

@nosami I've clarified that for Windows this only applies to non-ResX resources. For Linux, when using XBuild, the problem also applies to ResX resources.

Applying a fix for this on Windows would be a breaking change - do you see if there's a way to fix this without a breaking change?

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Apr 9, 2016

Contributor

@nosami I've put the specific issue about LogicalName here: #1050. That's probably the easiest thing to fix, though I looked around and could work out where to start

Contributor

dsyme commented Apr 9, 2016

@nosami I've put the specific issue about LogicalName here: #1050. That's probably the easiest thing to fix, though I looked around and could work out where to start

@nosami

This comment has been minimized.

Show comment
Hide comment
@nosami

nosami Apr 9, 2016

Member

Looking through the Xamarin code, it looks as if LogicalName is used by custom msbuild tasks rather than msbuild itself.

It will be a breaking change for Mono too, so not sure what the answer is here. It's quite a big issue at the moment for us, because our msbuild task (which references FSharp.Build.dll) ends up generating the same resource ID for different icon sizes for iOS (same filename in different folders) and we have no workaround. I'd rather that you adopted the correct behaviour, but would understand if you can't.

Being able to use LogicalName to override the generated name would at least give our end users a workaround. LogicalName is what we set when changing the resource ID in the IDE.

msbuild is currently in the bleeding edge mono builds so I'd expect it be adopted soon, probably the next public release.

Thanks!

Member

nosami commented Apr 9, 2016

Looking through the Xamarin code, it looks as if LogicalName is used by custom msbuild tasks rather than msbuild itself.

It will be a breaking change for Mono too, so not sure what the answer is here. It's quite a big issue at the moment for us, because our msbuild task (which references FSharp.Build.dll) ends up generating the same resource ID for different icon sizes for iOS (same filename in different folders) and we have no workaround. I'd rather that you adopted the correct behaviour, but would understand if you can't.

Being able to use LogicalName to override the generated name would at least give our end users a workaround. LogicalName is what we set when changing the resource ID in the IDE.

msbuild is currently in the bleeding edge mono builds so I'd expect it be adopted soon, probably the next public release.

Thanks!

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Apr 9, 2016

Contributor

It will be a breaking change for Mono too

Is that for XBuild or Mono more generally?

Contributor

dsyme commented Apr 9, 2016

It will be a breaking change for Mono too

Is that for XBuild or Mono more generally?

@nosami

This comment has been minimized.

Show comment
Hide comment
@nosami

nosami Apr 9, 2016

Member

It will only be a breaking change for us if a developer references the resource ID as a string in their code (and easy to fix).

Member

nosami commented Apr 9, 2016

It will only be a breaking change for us if a developer references the resource ID as a string in their code (and easy to fix).

@nosami

This comment has been minimized.

Show comment
Hide comment
@nosami

nosami Apr 9, 2016

Member

Is that for XBuild or Mono more generally?

XBuild or MSBuild delegates the job of generating resource IDs to the same msbuild task (that references this dll)

This issue affects F# only (xbuild or msbuild, Windows or Mono). C# is fine.

Member

nosami commented Apr 9, 2016

Is that for XBuild or Mono more generally?

XBuild or MSBuild delegates the job of generating resource IDs to the same msbuild task (that references this dll)

This issue affects F# only (xbuild or msbuild, Windows or Mono). C# is fine.

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Apr 9, 2016

Contributor

I'd rather that you adopted the correct behaviour, but would understand if you can't.

I believe we'd be happy to fix this, but I looked around and really couldn't find where the LogicalName information was consumed. It's likely an easy fix but I don't have any leads right now. If you find something please send a link!

Contributor

dsyme commented Apr 9, 2016

I'd rather that you adopted the correct behaviour, but would understand if you can't.

I believe we'd be happy to fix this, but I looked around and really couldn't find where the LogicalName information was consumed. It's likely an easy fix but I don't have any leads right now. If you find something please send a link!

@nosami

This comment has been minimized.

Show comment
Hide comment
@nosami

nosami Apr 9, 2016

Member

I've seen code, but it's in currently private Xamarin repos :)

All it does is use the name specified inside <LogicalName>resourceID</LogicalName> if it exists, otherwise generates the ID from the filename (including projectName and folder!)

Member

nosami commented Apr 9, 2016

I've seen code, but it's in currently private Xamarin repos :)

All it does is use the name specified inside <LogicalName>resourceID</LogicalName> if it exists, otherwise generates the ID from the filename (including projectName and folder!)

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Apr 9, 2016

Contributor

OK thanks. Please send any snippets you have. However I couldn't determine where the C# open source MSBuild code for this resides - that's really what we should be following.

Where would the corresponding fix go for F#? In Microsoft.FSharp.targets?

Contributor

dsyme commented Apr 9, 2016

OK thanks. Please send any snippets you have. However I couldn't determine where the C# open source MSBuild code for this resides - that's really what we should be following.

Where would the corresponding fix go for F#? In Microsoft.FSharp.targets?

@nosami

This comment has been minimized.

Show comment
Hide comment
@nosami

nosami Apr 9, 2016

Member

The mono code for this is here

ItemSpec comes from this function

Haven't seen any equivalent MS code, but this looks to generate the same ID as C# on MS .Net

Member

nosami commented Apr 9, 2016

The mono code for this is here

ItemSpec comes from this function

Haven't seen any equivalent MS code, but this looks to generate the same ID as C# on MS .Net

@nosami

This comment has been minimized.

Show comment
Hide comment
@nosami

nosami Apr 9, 2016

Member

I did find where this this fix should go once, but it's escaping me now - I should have linked to it when I created the issue!

Member

nosami commented Apr 9, 2016

I did find where this this fix should go once, but it's escaping me now - I should have linked to it when I created the issue!

nosami added a commit to nosami/visualfsharp that referenced this issue Jul 19, 2017

Make F# resources work more like C#
Given a resource in a folder named `AFolder/AResourceFile.txt` in a project with
a root namespace of `fsharpresources`, then the following command line arg
should be passed to the compiler

```
--resource:AFolder/AResourceFile.txt,fsharpresources.AFolder.AResourceFile.txt
```

whereas currently it just passes the name of the file. This also fixes passing
a `LogicalName` to the resource.

Fixes Microsoft#1050 &&
Microsoft#922

I looked at how Roslyn was doing this [here](https://github.com/dotnet/roslyn/blob/6847f1e5a909395aae9456e8f366cbf4deb86b69/src/Compilers/Core/MSBuildTask/ManagedCompiler.cs#L739)
es. Lines starting

nosami added a commit to nosami/fsharp that referenced this issue Jul 19, 2017

Make F# resources work more like C#
Given a resource in a folder named `AFolder/AResourceFile.txt` in a project with
a root namespace of `fsharpresources`, then the following command line arg
should be passed to the compiler

```
--resource:AFolder/AResourceFile.txt,fsharpresources.AFolder.AResourceFile.txt
```

whereas currently it just passes the name of the file. This also fixes passing
a `LogicalName` to the resource.

Fixes Microsoft/visualfsharp#1050 &&
Microsoft/visualfsharp#922

I looked at how Roslyn was doing this [here](https://github.com/dotnet/roslyn/blob/6847f1e5a909395aae9456e8f366cbf4deb86b69/src/Compilers/Core/MSBuildTask/ManagedCompiler.cs#L739)
es. Lines starting

KevinRansom added a commit that referenced this issue Jul 25, 2017

Make F# resources work more like C# (#3352)
* Make F# resources work more like C#

Given a resource in a folder named `AFolder/AResourceFile.txt` in a project with
a root namespace of `fsharpresources`, then the following command line arg
should be passed to the compiler

```
--resource:AFolder/AResourceFile.txt,fsharpresources.AFolder.AResourceFile.txt
```

whereas currently it just passes the name of the file. This also fixes passing
a `LogicalName` to the resource.

Fixes #1050 &&
#922

I looked at how Roslyn was doing this [here](https://github.com/dotnet/roslyn/blob/6847f1e5a909395aae9456e8f366cbf4deb86b69/src/Compilers/Core/MSBuildTask/ManagedCompiler.cs#L739)
es. Lines starting

* Add a UseStandardResourceNames flag (disabled by default)

With
```
    <UseStandardResourceNames>true</UseStandardResourceNames>
```
we get the following output

```
--resource:AFolder/AResourceFile.txt,fsharpresource.AFolder.AResourceFile.txt
```

With the value omitted or set to false, the behaviour is as it was previously.

```
--resource:AFolder/AResourceFile.txt
```
@ForNeVeR

This comment has been minimized.

Show comment
Hide comment
@ForNeVeR

ForNeVeR Dec 17, 2017

Contributor

@KevinRansom looks like this issue was intended to be closed by commit f6049d5, right?

Contributor

ForNeVeR commented Dec 17, 2017

@KevinRansom looks like this issue was intended to be closed by commit f6049d5, right?

@cartermp cartermp closed this Dec 17, 2017

@KevinRansom

This comment has been minimized.

Show comment
Hide comment
@KevinRansom

KevinRansom Dec 18, 2017

Contributor

@ForNeVeR yes it seems like it.

Contributor

KevinRansom commented Dec 18, 2017

@ForNeVeR yes it seems like it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment