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

WIP: Single resx compilation using dotnet cli for compilation #96

Closed
wants to merge 2 commits into from
Closed

WIP: Single resx compilation using dotnet cli for compilation #96

wants to merge 2 commits into from

Conversation

jrbeverly
Copy link
Contributor

An API for compiling a single resx into a .resources binary.

Related to #10

@jrbeverly jrbeverly requested review from j3parker and omsmith October 15, 2019 18:41
@jrbeverly jrbeverly self-assigned this Oct 15, 2019
print("todo")

csharp_resx = rule(
implementation = _csharp_resx_impl,
Copy link
Contributor Author

@jrbeverly jrbeverly Oct 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking we'll want to provide a custom struct as mentioned in #77, as the name of the output .resources file is determined by either the csproj file name or the RootNamespace property. We could add an out property for modifying the name of the csproj file (rather than relying solely on identifier) (more on PR #93)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The struct sounds good. I think the fact that we're generating a csproj behind the scenes is an implementation detail that we wouldn't want to expose via attrs though?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The struct containing a identifier property (or out for controlling output name) is not really related to the csproj, but to how the resources are consumed by C# compiler:

-resource:filename[,identifier[,accessibility-modifier]]

With parameters like that, I can pass in a resources file named hello.look.at.this.name.resources, but use the identifier hello.Strings.resources. This is what I was getting at in this discussion

csharp_resx = rule(
implementation = _csharp_resx_impl,
attrs = {
"src": attr.label(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be more ergonomic for this to be srcs and to invoke dotnet multiple times in a loop behind the scenes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we went this route, the API for specifying the identifier would need to be a label_keyed_string_dict, which makes the specification of the identifier mandatory.

I would also have concerns with the resource being used incorrectly with localized resx files. For localized resx files, satellite assemblies are created in directories based on the culture code. Those will need some special handling, as the pathing of those assemblies matter.

default = Label(_TEMPLATE),
allow_single_file = True,
),
"_dotnet_runner": attr.string(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should use toolchains instead. Check out how the csharp_library etc. rules get access to dotnet.exe

deps = ["@net//:System.Linq"],
)

csharp_resx(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. If src -> srcs then it'd be idiomatic to name this rule csharp_resx_library.

@j3parker
Copy link
Member

Some of the errors in CI are about adding the symbol to https://github.com/Brightspace/rules_csharp/blob/master/csharp/defs.bzl

mandatory = True,
allow_single_file = True
),
"identifier": attr.string(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been looking into an out parameter to specify the name of the compiled resource. With EmbeddedResource's you can specify the property LogicalName that is used by GenerateResource to set the output name.

The GenerateResource task uses the metadata of an item to name the resource that is embedded in an assembly.

Assuming that the assembly is named myAssembly, the following code generates an embedded resource named someQualifier.someResource.resources:

Based on my testing so far I haven't been successful with getting the compiled resource named based on the LogicalName property. I'll try testing this from visual studio, to get a better idea of how this is happening in that environment.

@jrbeverly
Copy link
Contributor Author

Closing draft as working copy is now raised at #98.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants