-
Notifications
You must be signed in to change notification settings - Fork 369
Improved C# support #1258
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
base: main
Are you sure you want to change the base?
Improved C# support #1258
Changes from all commits
dab0f7a
c2e8367
55f33ef
3abbbcd
7754471
4b237e0
54d4c12
8ab8c8f
4060394
d58d501
69df7d5
e83525e
215f649
fd7b704
7a745b0
f5496e6
9864ad3
af57649
9c8007a
567466f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| # Copyright (c) Meta Platforms, Inc. and affiliates. | ||
| # | ||
| # This source code is dual-licensed under either the MIT license found in the | ||
| # LICENSE-MIT file in the root directory of this source tree or the Apache | ||
| # License, Version 2.0 found in the LICENSE-APACHE file in the root directory | ||
| # of this source tree. You may select, at your option, one of the | ||
| # above-listed licenses. | ||
|
|
||
| # TODO(cjhopman): This was generated by scripts/hacks/rules_shim_with_docs.py, | ||
| # but should be manually edited going forward. There may be some errors in | ||
| # the generated docs, and so those should be verified to be accurate and | ||
| # well-formatted (and then delete this TODO) | ||
|
|
||
| FrameworkVersion = ["net35", "net40", "net45", "net46"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,8 +12,32 @@ | |
| # well-formatted (and then delete this TODO) | ||
|
|
||
| load(":common.bzl", "buck", "prelude_rule") | ||
| load(":dotnet_common.bzl", "FrameworkVersion") | ||
| load("@prelude//csharp:csharp_providers.bzl", "DotNetLibraryInfo") | ||
|
|
||
| FrameworkVersion = ["net35", "net40", "net45", "net46"] | ||
| _CSHARP_LIBRARY_OR_EXE_ATTRIBUTES = { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of the comments below are almost all just improvements to what was already pre-existing. While we can fix it, we should. If you're not certain about this, when we go to merge it I can make edits, too.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I've fixed all the issues except for the |
||
| "srcs": attrs.list(attrs.source(), default = [], doc = """ | ||
| The set of C# source files to be compiled, and assembled by this rule. | ||
| Each element may be either a literal string (representing the path within this package), or a target. | ||
| """), | ||
| "framework_ver": attrs.enum(FrameworkVersion, doc = """ | ||
| The version of the .Net framework that this library targets. This is | ||
| one of """ + ", ".join(FrameworkVersion) + """. | ||
| """), | ||
| "deps": attrs.list(attrs.one_of(attrs.dep(providers = [DotNetLibraryInfo]), attrs.string()), default = [], doc = """ | ||
| The set of targets or system-provided assemblies this target depends on. Any | ||
| values that are targets must be either csharp\\_library or `prebuilt_dotnet_library` | ||
| instances. | ||
| """), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "to rely on" => "this target depends on." What does it mean for this to be an If you depend on a particular provider being in here, your
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The attrs.string is for "system-provided assemblies" like System.IO.dll |
||
| "compiler_flags": attrs.list(attrs.arg(), default = [], doc = """ | ||
| The set of additional compiler flags. | ||
| """), | ||
| "add_hermetic_arguments": attrs.bool(default = True, doc = """ | ||
| If true, the following arguments are passed to the compiler: "/noconfig", "/nostdlib" and "/nosdkpath". | ||
| These attributes prevent loading of default assemblies by the compiler so that they can be explicitly | ||
| controlled in Buck2. | ||
| """), | ||
| } | ||
|
|
||
| csharp_library = prelude_rule( | ||
| name = "csharp_library", | ||
|
|
@@ -22,19 +46,14 @@ csharp_library = prelude_rule( | |
| and dependencies by invoking csc. | ||
| """, | ||
| examples = """ | ||
| For more examples, check out our [integration tests](https://github.com/facebook/buck/tree/dev/test/com/facebook/buck/rust/testdata/). | ||
|
|
||
| ``` | ||
| ```python | ||
| csharp_library( | ||
| name = 'simple', | ||
| dll_name = 'Cake.dll', | ||
| framework_ver = 'net46', | ||
| srcs = [ | ||
| 'Hello.cs', | ||
| ], | ||
| resources = { | ||
| 'greeting.txt': '//some:target', | ||
| }, | ||
| deps=[ | ||
| ':other', | ||
| 'System.dll', | ||
|
|
@@ -57,65 +76,67 @@ csharp_library = prelude_rule( | |
| The output name of the dll. This allows you to specify the name of | ||
| the dll exactly. When this is not set, the dll will be named after | ||
| the short name of the target. | ||
| """, | ||
| ), | ||
| "srcs": attrs.list( | ||
| attrs.source(), | ||
| default = [], | ||
| doc = """ | ||
| The collection of source files to compile. | ||
| """, | ||
| ), | ||
| "resources": attrs.dict( | ||
| key = attrs.string(), | ||
| value = attrs.source(), | ||
| sorted = False, | ||
| default = {}, | ||
| doc = """ | ||
| Resources that should be embedded within the built DLL. The format | ||
| is the name of the resource once mapped into the DLL as the key, and | ||
| the value being the resource that should be merged. This allows | ||
| non-unique keys to be identified quickly. | ||
| """, | ||
| ), | ||
| "framework_ver": attrs.enum( | ||
| FrameworkVersion, | ||
| doc = """ | ||
| The version of the .Net framework that this library targets. This is | ||
| one of 'net35', 'net40', 'net45' and 'net46'. | ||
| """, | ||
| ), | ||
| "deps": attrs.list( | ||
| attrs.one_of(attrs.dep(), attrs.string()), | ||
| default = [], | ||
| doc = """ | ||
| The set of targets or system-provided assemblies to rely on. Any | ||
| values that are targets must be either csharp\\_library or `prebuilt_dotnet_library` | ||
| instances. | ||
| """, | ||
| ), | ||
| "compiler_flags": attrs.list( | ||
| attrs.string(), | ||
| default = [], | ||
| doc = """ | ||
| The set of additional compiler flags to pass to the compiler. | ||
| """, | ||
| ), | ||
| """), | ||
| } | ||
| | _CSHARP_LIBRARY_OR_EXE_ATTRIBUTES | ||
| | buck.licenses_arg() | ||
| | buck.labels_arg() | ||
| | buck.contacts_arg() | ||
| ), | ||
| ) | ||
|
|
||
| csharp_binary = prelude_rule( | ||
| name = "csharp_binary", | ||
| docs = """ | ||
| A csharp\\_binary() rule builds a .Net library from the supplied set of C# source files | ||
| and dependencies by invoking csc. | ||
| """, | ||
| examples = """ | ||
| ```python | ||
| csharp_binary( | ||
| name = 'simple', | ||
| exe_name = 'Cake.exe', | ||
| framework_ver = 'net46', | ||
| srcs = [ | ||
| 'Hello.cs', | ||
| ], | ||
| deps=[ | ||
| ':other', | ||
| 'System.dll', | ||
| ], | ||
| ) | ||
|
|
||
| prebuilt_dotnet_library( | ||
| name = 'other', | ||
| assembly = 'other-1.0.dll', | ||
| ) | ||
| ``` | ||
| """, | ||
| further = None, | ||
| attrs = ( | ||
| # @unsorted-dict-items | ||
| { | ||
| "exe_name": attrs.string(default = "", doc = """ | ||
| The output name of the executable. This allows you to specify the name of | ||
| the executable exactly. When this is not set, the executable will be named after | ||
| the short name of the target. | ||
| """), | ||
| } | | ||
| _CSHARP_LIBRARY_OR_EXE_ATTRIBUTES | | ||
| buck.licenses_arg() | | ||
| buck.labels_arg() | | ||
| buck.contacts_arg() | ||
| ), | ||
| ) | ||
|
|
||
| prebuilt_dotnet_library = prelude_rule( | ||
| name = "prebuilt_dotnet_library", | ||
| docs = """ | ||
| A `prebuilt_dotnet_library()` rule is used to include | ||
| prebuilt .Net assembles into your .Net code. | ||
| """, | ||
| examples = """ | ||
| ``` | ||
| ```python | ||
| prebuilt_dotnet_library( | ||
| name = 'log4net', | ||
| assembly = 'log4net.dll', | ||
|
|
@@ -152,5 +173,6 @@ prebuilt_dotnet_library = prelude_rule( | |
|
|
||
| dotnet_rules = struct( | ||
| csharp_library = csharp_library, | ||
| csharp_binary = csharp_binary, | ||
| prebuilt_dotnet_library = prebuilt_dotnet_library, | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a hazard here that can't be directly fixed. In a very large project, you may have MANY entries here, and the size of this can blow up. You do have to enumerate these at some point and create the symlinks, though.
Another approach is to farm out the symlink creation to a script/tool/whatever that would create all the symlinks for you, and instead of calling
child.traverse()here, you'd give that script the result of a "projection" on the transitive set. In your implementation,new_runtime_fileswill be the size of the graph represented by the TSet. In the other approach, the projection is just a refernce to the TSet root that says "run the X projection", and the TSet is only walked when the action that runs the script is run. Thepython_binaryrules do this sort of thing to avoid building these (huge) lists in prelude/python/python.bzl, in theargs_projections. These are later used to build manifest files or symlink trees (like you're doing here) with the help of external commands that take the list of arguments as inputs in some form. The python rules used.traverse()at one time, consuming single-digit gigabytes of memory for eachpython_binarytarget.This isn't functionally wrong, but anywhere you can avoid copying the contents of a TSet is an opportunity to not waste memory. Because artifacts need to be declared, doing this work with
.symlink_filehas to do this, I think. I don't believe there's a way currently to say "build up a bunch of symlinks here by walking this TSet", hence the external tools that take the list of symlinks as arguments in some form and create them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figured the optimal way would be to avoid traverse but I didn't see a way of doing that. I guess using a separate tool would work but it seems harder to maintain rather than using Starlark's native symlink_file.
Could we merge this like this and if people start raising issues with this taking a lot of memory we can implement this external tool improvement? With any luck by that time there will be a better way of doing this.