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

Import Roslyn-tools SignTool into Arcade #266

Closed
JohnTortugo opened this issue Jun 19, 2018 · 9 comments
Closed

Import Roslyn-tools SignTool into Arcade #266

JohnTortugo opened this issue Jun 19, 2018 · 9 comments
Assignees

Comments

@JohnTortugo
Copy link
Contributor

No description provided.

@JohnTortugo JohnTortugo self-assigned this Jun 19, 2018
@weshaggard
Copy link
Member

While porting over the signing stuff we will need to incorporate the aggregate of the strong names defined in:
https://github.com/dotnet/roslyn-tools/blob/0bce8ad78a0afe6d03e6ba95c3a9e9729cbc00c9/src/RepoToolset/tools/StrongName.targets
https://github.com/dotnet/buildtools/blob/master/src/Microsoft.DotNet.Build.Tasks/PackageFiles/sign.targets

We should also figure out how to reconcile the need for the checked in signtooldata file, see #58.

We should also highly consider converting the signtool to an msbuild task instead of a separate exe like it is today. It already has to depend on msbuild see (https://github.com/dotnet/roslyn-tools/blob/0bce8ad78a0afe6d03e6ba95c3a9e9729cbc00c9/src/RepoToolset/tools/Sign.proj#L36) so why not just use the msbuild we are using everywhere else. At a min we ensure we don't require VS and the version of msbuild that comes with it like is currently happening. https://github.com/dotnet/roslyn-tools/blob/0bce8ad78a0afe6d03e6ba95c3a9e9729cbc00c9/src/RepoToolset/tools/Sign.proj#L23

@tmat any reason the signtool isn't an msbuild task?

@JohnTortugo
Copy link
Contributor Author

Just an update: as per talk with @maririos she told me that the SignTool conversion to MSBuild Task was discussed with @tmat and he agreed to it.

@jaredpar
Copy link
Member

@weshaggard

any reason the signtool isn't an msbuild task?

No particular reason. When I wrote SignTool originally it just fit better as an EXE into our build than a build task.

We should also highly consider converting the signtool to an msbuild task instead of a separate exe like it is today.

Don't have any objections to adding an MSBuild task but would prefer to keep the EXE. It's handy in a number of cases.

We should also figure out how to reconcile the need for the checked in signtooldata file, see #58.

Still not sure what we should reconcile here. The only people objecting to a checked in file are those who haven't used the tool. There are no complaints from those who are using it (and there are plenty of users now).

@weshaggard
Copy link
Member

Still not sure what we should reconcile here. The only people objecting to a checked in file are those who haven't used the tool. There are no complaints from those who are using it (and there are plenty of users now).

Lets continue this discussion in #58.

@rainersigwald
Copy link
Member

Note that you can load a task from any managed assembly, even a .exe, so one could conceivably just add a class to the existing executable and load it from there to keep both.

@JohnTortugo
Copy link
Contributor Author

@weshaggard & @jaredpar after @rainersigwald comment, do you agree to keep the .exe and create a MSBuild target to wrap it? If so, I could start moving this into Arcade.

@weshaggard
Copy link
Member

I don't know enough about how it is currently implemented to say if that is the best option or not. I do agree that as much of the logic as possible should be shared. One challenge you will likely have keeping it only as an exe is that we will need a task library that can be loaded into .NET Core msbuild and exe's are generally .NET Framework and cannot be loaded in .NET Core because of their dependencies.

@maririos
Copy link
Member

I created issue #273 to use it for the discussion about convert or not convert the signtool to an msbuild task. Once that is clear, we can proceed on the implementation.

@maririos
Copy link
Member

Also, created issue #272 for the strong names work.

Note that all of the issues are now in the Signing Package Ready for Consumption epic

@JohnTortugo JohnTortugo changed the title Include Signing Package on Arcade SDK Import Roslyn-tools SignTool into Arcade Jun 27, 2018
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

No branches or pull requests

5 participants