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

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

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

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

nosami opened this issue Feb 1, 2016 · 17 comments

Comments

@nosami
Copy link
Contributor

@nosami 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
Copy link
Contributor

@mexx 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
Copy link
Contributor

@dsyme 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
Copy link
Contributor Author

@nosami 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
Copy link
Contributor

@dsyme 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 Incorrect resource IDs generated by FSharp.Build Non-ResX resources get different resource IDs on Windows compared to C# Apr 9, 2016
@dsyme
Copy link
Contributor

@dsyme 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
Copy link
Contributor

@dsyme 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
Copy link
Contributor Author

@nosami 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
Copy link
Contributor

@dsyme dsyme commented Apr 9, 2016

It will be a breaking change for Mono too

Is that for XBuild or Mono more generally?

@nosami
Copy link
Contributor Author

@nosami 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
Copy link
Contributor Author

@nosami 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
Copy link
Contributor

@dsyme 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
Copy link
Contributor Author

@nosami 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
Copy link
Contributor

@dsyme 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
Copy link
Contributor Author

@nosami 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
Copy link
Contributor Author

@nosami 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
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 dotnet#1050 &&
dotnet#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
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 dotnet/fsharp#1050 &&
dotnet/fsharp#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#

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
Copy link
Contributor

@ForNeVeR 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
Copy link
Member

@KevinRansom 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
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.