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.

Sign up for GitHub

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

Jump to bottom

define @net//:StandardReferences #82

Merged
merged 2 commits into from
Oct 3, 2019
Merged

define @net//:StandardReferences #82

merged 2 commits into from
Oct 3, 2019

Conversation

omsmith
Copy link
Contributor

@omsmith omsmith commented Oct 2, 2019 •
edited
Loading

Closes: #41

  • Blocked on EnterpriseServices.Wrapper

@omsmith
Copy link
Contributor Author

omsmith commented Oct 2, 2019 •
edited
Loading

Can successfully build netstandard* and netcoreapp*, but not framework

/home/omsmith/dev/rules_csharp/examples/every_framework_empty ~ bazel build :net20
INFO: Analyzed target //every_framework_empty:net20 (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
ERROR: /home/omsmith/dev/rules_csharp/examples/every_framework_empty/BUILD:6:1: Compiling net20 failed (Exit 1) dotnet failed: error executing command external/netcore-runtime-linux/dotnet external/csharp-build-tools/tasks/netcoreapp2.1/bincore/csc.dll /noconfig /unsafe- /checked- /nostdlib+ /utf8output /deterministic+ /filealign:512 /nologo ... (remaining 23 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
warning CS2008: No source files specified.
error CS0006: Metadata file '/home/omsmith/.cache/bazel/_bazel_omsmith/2fb55c7c47f7f9f7347244ac3cb377ea/sandbox/linux-sandbox/20/execroot/__main__/external/net20/build/.NETFramework/v2.0/System.EnterpriseServices.Wrapper.dll' could not be found
Target //every_framework_empty:net20 failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 0.609s, Critical Path: 0.56s
INFO: 0 processes.
FAILED: Build did NOT complete successfully

j3parker
j3parker reviewed Oct 2, 2019
View reviewed changes
csharp/private/rules/binary.bzl
@@ -99,7 +99,7 @@ csharp_binary = rule(
),
"_stdrefs": attr.label(
doc = "The standard set of assemblies to reference.",
default = "@net//:mscorlib", # TODO: change to @net//:StandardReferences once it exists
default = "@net//:StandardReferences",
Copy link
Member

Choose a reason for hiding this comment

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

So good!

j3parker
j3parker approved these changes Oct 2, 2019
View reviewed changes
Copy link
Member

@j3parker j3parker left a comment

Choose a reason for hiding this comment

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

I wish I could approve this twice!

j3parker
j3parker previously approved these changes Oct 2, 2019
View reviewed changes
Copy link
Member

@j3parker j3parker left a comment

Choose a reason for hiding this comment

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

Oh, you can!

omsmith
omsmith commented Oct 3, 2019
View reviewed changes
csharp/private/actions/assembly.bzl Outdated
@@ -183,6 +183,7 @@ def AssemblyAction(
out = out_file,
refout = refout,
pdb = pdb,
native_dlls = None,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, this is obviously wrong (as evidenced by CI). I just got as for as getting bazel build :net20 in every_framework_empty happy

@omsmith omsmith force-pushed the standard-references branch from 9829f0d to 46d6757 Compare October 3, 2019 15:59
j3parker
j3parker previously approved these changes Oct 3, 2019
View reviewed changes
@j3parker
Copy link
Member

j3parker commented Oct 3, 2019

So there are native DLLs that I think we just want to show up in runfiles (e.g. libgit2) but not at compile time. This one is weird because we don't want it in runfiles (it'll come from the GAC in practice) but we do need it at compile time...

I think it's reasonable to do this now but we'll probably need to revisit things later. I'll open up an issue to track this.

@j3parker j3parker mentioned this pull request Oct 3, 2019
Open
@omsmith omsmith force-pushed the standard-references branch 2 times, most recently from 707c664 to 2879793 Compare October 3, 2019 17:47
@omsmith omsmith force-pushed the standard-references branch from 2879793 to b149056 Compare October 3, 2019 17:53
@omsmith omsmith merged commit 66c58e6 into Brightspace:master Oct 3, 2019
@omsmith omsmith deleted the standard-references branch October 3, 2019 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@j3parker j3parker j3parker approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add default references like System
2 participants
@omsmith @j3parker