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

Use /publicsign instead of FakeSign #7747

Merged
merged 5 commits into from
Jan 3, 2016
Merged

Conversation

agocke
Copy link
Member

@agocke agocke commented Dec 31, 2015

Replace usage of FakeSign with /publicsign in Roslyn.

@dotnet/roslyn-compiler @dotnet/roslyn-infrastructure @jaredpar

The compiler now supports publicsign so FakeSign isn't necessary.
However, since we're not using the toolset compiler on *nix we can't
using the PublicSign property. In the meantime we can pass publicsign in
a response file.
Brings in a new toolset compiler that supports publicsign in VB and
updates the ci bootstrap to use the bootstrap build task, which
provides bootstrap support for the <PublicSign> property.
@agocke
Copy link
Member Author

agocke commented Dec 31, 2015

Forgot to update the nuget package.

@agocke
Copy link
Member Author

agocke commented Jan 2, 2016

test prtest/win/dbg/unit64 please

built against the xplat MSBuild references so it can't be loaded properly. Providing
a response file works around the problem by directly adding the public sign argument
to all unix compilations. This shouldn't present a problem as it's impossible to
fully sign on unix at the moment anyway -->
Copy link
Member

Choose a reason for hiding this comment

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

Where are the bugs tracking these issues? Seems like there are two items to track here:

  1. Removing this work around on unix.
  2. Removing the MSBuild limitation that prevents us from using the <PublicSign> property.

The latter is particularly interesting because it will presumably affect other items like determinism.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this is kind of nasty. I've filed #7756 to track removing this and I'll add a mention of the bug number in the comment.

There are some underlying infrastructrue issues here. First, it appears the MSBuild on Mono eagerly loads tasks listed in Microsoft.Common.tasks. That means that the default task is loaded before we have a chance to redirect it to anywhere else.

But even if that were fixed (which would be difficult, I think, because I don't think the MSBuild version for Mono is getting any updates) there would still be another issue -- our build task is currently built against the Windows desktop MSBuild references. Theoretically, we could dig through the Mono MSBuild NuGet package, find the references, and build a third build task against those references.

I think that's getting a bit ridiculous though -- we already have a build task built against the CoreCLR MSBuild references that's actively being developed. We should instead start moving forward to using that. I've filed bug #7755 for that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might be related a bug I filed years ago... I have given up on supporting Mono due to them being lazy monkeys (not offensive, but english for mono) for not fixing non-commercial (read Xamarin) bugs...

Copy link
Contributor

Choose a reason for hiding this comment

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

@jaredpar
Copy link
Member

jaredpar commented Jan 3, 2016

👍

agocke added a commit that referenced this pull request Jan 3, 2016
Use /publicsign instead of FakeSign
@agocke agocke merged commit 4c10994 into dotnet:master Jan 3, 2016
@agocke agocke deleted the RemoveFakeSign branch January 3, 2016 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants