Skip to content

Commit

Permalink
Make F# resources work more like C# (#3352)
Browse files Browse the repository at this point in the history
* 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
```
  • Loading branch information
nosami authored and KevinRansom committed Jul 25, 2017
1 parent 7af8b4a commit f6049d5
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 22 deletions.
38 changes: 18 additions & 20 deletions src/fsharp/FSharp.Build/CreateFSharpManifestResourceName.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ open Microsoft.Build.Utilities

type CreateFSharpManifestResourceName public () =
inherit CreateCSharpManifestResourceName()


// When set to true, generate resource names in the same way as C# with root namespace and folder names
member val UseStandardResourceNames = false with get, set

override this.CreateManifestName
((fileName:string),
(linkFileName:string),
Expand All @@ -23,34 +26,29 @@ type CreateFSharpManifestResourceName public () =
//
// For resx resources, both the Visual FSharp and XBuild FSHarp toolchains do the right thing, i.e.
// SubDir\abc.resx --> SubDir.abc.resources
//
// However for non-resx resources, for some reason Visual FSharp does _not_ add the directory name to the resource name.
// It is very unclear where the directory name gets dropped in the Visual FSharp implementation
// - is it in Microsoft.Common.targets, Microsoft.FSharp.Targets or how the base type CreateCSharpManifestResourceName
// is created and used - who knows, the code is not easy to understand despite it doing something very simple. That's
// the nature of MSBuild/XBuild....
//
// Anyway, dropping the directory name seems like a mistake. But we attempt to replicate the behaviour here
// for consistency with Visual FSharp. This may not be the right place to do this and this many not be consistent
// when cultures are used - that case has not been tested.

let runningOnMono =
try
System.Type.GetType("Mono.Runtime") <> null
with e ->
false
let fileName = if not runningOnMono || fileName.EndsWith(".resources", StringComparison.OrdinalIgnoreCase) then fileName else Path.GetFileName(fileName)
let linkFileName = if not runningOnMono || linkFileName.EndsWith(".resources", StringComparison.OrdinalIgnoreCase) then linkFileName else Path.GetFileName(linkFileName)
let fileName, linkFileName, rootNamespace =
match this.UseStandardResourceNames with
| true ->
fileName, linkFileName, rootNamespace
| false ->
let runningOnMono =
try
System.Type.GetType("Mono.Runtime") <> null
with e ->
false
let fileName = if not runningOnMono || fileName.EndsWith(".resources", StringComparison.OrdinalIgnoreCase) then fileName else Path.GetFileName(fileName)
let linkFileName = if not runningOnMono || linkFileName.EndsWith(".resources", StringComparison.OrdinalIgnoreCase) then linkFileName else Path.GetFileName(linkFileName)
fileName, linkFileName, ""

let embeddedFileName =
match linkFileName with
| null -> fileName
| _ -> linkFileName

// since we do not support resources dependent on a form, we always pass null for a binary stream
// rootNamespace is always empty - we do not support it
let cSharpResult =
base.CreateManifestName(fileName, linkFileName, "", dependentUponFileName, null)
base.CreateManifestName(fileName, linkFileName, rootNamespace, dependentUponFileName, null)
// Workaround that makes us keep .resources extension on both 3.5 and 3.5SP1
// 3.5 stripped ".resources", 3.5 SP1 does not. We should do 3.5SP1 thing
let extensionToWorkaround = ".resources"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ namespace Microsoft.FSharp.Build
type CreateFSharpManifestResourceName =
inherit Microsoft.Build.Tasks.CreateCSharpManifestResourceName
public new : unit -> CreateFSharpManifestResourceName
member UseStandardResourceNames : bool with get,set
19 changes: 17 additions & 2 deletions src/fsharp/FSharp.Build/Fsc.fs
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,17 @@ type FscCommandLineBuilder () =
if s <> String.Empty then
args <- s :: args

member x.AppendSwitchIfNotNull(switch:string, value:string) =
member x.AppendSwitchIfNotNull(switch:string, value:string, ?metadataNames:string array) =
let metadataNames = defaultArg metadataNames [||]
builder.AppendSwitchIfNotNull(switch, value)
let tmp = new CommandLineBuilder()
tmp.AppendSwitchUnquotedIfNotNull(switch, value)
let providedMetaData =
metadataNames
|> Array.filter (String.IsNullOrWhiteSpace >> not)
if providedMetaData.Length > 0 then
tmp.AppendTextUnquoted ","
tmp.AppendTextUnquoted (providedMetaData|> String.concat ",")
let s = tmp.ToString()
if s <> String.Empty then
args <- s :: args
Expand Down Expand Up @@ -161,6 +168,7 @@ type [<Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1704:Iden
| Some s -> s
| None -> ""
let mutable treatWarningsAsErrors : bool = false
let mutable useStandardResourceNames : bool = false
let mutable warningsAsErrors : string = null
let mutable versionFile : string = null
let mutable warningLevel : string = null
Expand Down Expand Up @@ -243,7 +251,10 @@ type [<Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1704:Iden
// Resources
if resources <> null then
for item in resources do
builder.AppendSwitchIfNotNull("--resource:", item.ItemSpec)
match useStandardResourceNames with
| true -> builder.AppendSwitchIfNotNull("--resource:", item.ItemSpec, [|item.GetMetadata("LogicalName"); item.GetMetadata("Access")|])
| false -> builder.AppendSwitchIfNotNull("--resource:", item.ItemSpec)

// VersionFile
builder.AppendSwitchIfNotNull("--versionfile:", versionFile)
// References
Expand Down Expand Up @@ -514,6 +525,10 @@ type [<Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1704:Iden
with get() = toolPath
and set(s) = toolPath <- s

// When set to true, generate resource names in the same way as C# with root namespace and folder names
member fsc.UseStandardResourceNames
with get() = useStandardResourceNames
and set(s) = useStandardResourceNames <- s
// --version-file <string>:
member fsc.VersionFile
with get() = versionFile
Expand Down
1 change: 1 addition & 0 deletions src/fsharp/FSharp.Build/Fsc.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ type Fsc = class
member ToolPath : string with get,set
member TargetProfile : string with get,set
member TreatWarningsAsErrors : bool with get,set
member UseStandardResourceNames : bool with get,set
member Utf8Output : bool with get,set
member VisualStudioStyleErrors : bool with get,set
member WarningLevel : string with get,set
Expand Down
7 changes: 7 additions & 0 deletions src/fsharp/FSharp.Build/Microsoft.FSharp.Targets
Original file line number Diff line number Diff line change
Expand Up @@ -89,24 +89,28 @@ this file.

<CreateFSharpManifestResourceName
Condition="'@(ResxWithNoCulture)' != '' AND '$(UsingXBuild)' == 'true'"
UseStandardResourceNames="$(UseStandardResourceNames)"
ResourceFiles="@(ResxWithNoCulture)" RootNamespace="$(RootNamespace)">
<Output TaskParameter = "ManifestResourceNames" ItemName = "ManifestResourceWithNoCultureName" />
</CreateFSharpManifestResourceName>

<CreateFSharpManifestResourceName
Condition="'@(NonResxWithNoCulture)' != '' AND '$(UsingXBuild)' == 'true'"
UseStandardResourceNames="$(UseStandardResourceNames)"
ResourceFiles="@(NonResxWithNoCulture)" RootNamespace="$(RootNamespace)">
<Output TaskParameter = "ManifestResourceNames" ItemName = "ManifestNonResxWithNoCulture" />
</CreateFSharpManifestResourceName>

<CreateFSharpManifestResourceName
Condition="'@(ResxWithCulture)' != '' AND '$(UsingXBuild)' == 'true'"
UseStandardResourceNames="$(UseStandardResourceNames)"
ResourceFiles="@(ResxWithCulture)" RootNamespace="$(RootNamespace)">
<Output TaskParameter = "ManifestResourceNames" ItemName = "ManifestResourceWithCultureName" />
</CreateFSharpManifestResourceName>

<CreateFSharpManifestResourceName
Condition="'@(NonResxWithCulture)' != '' AND '$(UsingXBuild)' == 'true'"
UseStandardResourceNames="$(UseStandardResourceNames)"
ResourceFiles="@(NonResxWithCulture)" RootNamespace="$(RootNamespace)">
<Output TaskParameter = "ManifestResourceNames" ItemName = "ManifestNonResxWithCulture" />
</CreateFSharpManifestResourceName>
Expand All @@ -123,6 +127,7 @@ this file.
<CreateFSharpManifestResourceName
ResourceFiles="@(EmbeddedResource)"
RootNamespace="$(RootNamespace)"
UseStandardResourceNames="$(UseStandardResourceNames)"
Condition="'%(EmbeddedResource.ManifestResourceName)' == '' and ('%(EmbeddedResource.WithCulture)' == 'false' or '%(EmbeddedResource.Type)' == 'Resx') AND '$(UsingXBuild)' == 'false'">

<Output TaskParameter="ResourceFilesWithManifestResourceNames" ItemName="_Temporary" />
Expand All @@ -133,6 +138,7 @@ this file.
<CreateFSharpManifestResourceName
ResourceFiles="@(EmbeddedResource)"
RootNamespace="$(RootNamespace)"
UseStandardResourceNames="$(UseStandardResourceNames)"
PrependCultureAsDirectory="false"
Condition="'%(EmbeddedResource.ManifestResourceName)' == '' and '%(EmbeddedResource.WithCulture)' == 'true' and '%(EmbeddedResource.Type)' == 'Non-Resx' AND '$(UsingXBuild)' == 'false'">

Expand Down Expand Up @@ -259,6 +265,7 @@ this file.
ToolExe="$(FscToolExe)"
ToolPath="$(FscToolPath)"
TreatWarningsAsErrors="$(TreatWarningsAsErrors)"
UseStandardResourceNames="$(UseStandardResourceNames)"
Utf8Output="$(Utf8Output)"
VersionFile="$(VersionFile)"
VisualStudioStyleErrors="$(VisualStudioStyleErrors)"
Expand Down

0 comments on commit f6049d5

Please sign in to comment.