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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #238 - Signed Google.Apis #550

Merged
merged 8 commits into from Jul 7, 2015
Merged

Issue #238 - Signed Google.Apis #550

merged 8 commits into from Jul 7, 2015

Conversation

@peleyal
Copy link
Collaborator

peleyal commented Jun 12, 2015

The change contains the following:

  1. Change Google.Apis projects to support a new ReleaseSigned configuraiton
  2. Change the publisher and the release process to use the ReleaseSigned version (and not Release)
  3. Change the release process to use Git (and not HG...)
  4. Add the google.apis.public.snk key
  • It was tested with a beta version (but signed) of Zlib.Portable. Now waiting for them to publish a new stable version.
    UPDATE: Zlib.Portable already posted a new signed (not beta) version of their library. 馃憤

I followed Linda instructions;
The key was created using the following:
sn.exe -k asecret.snk
Getting the public key from the secret:
sn.exe -p asecret.snk publicKey.snk
Then I added the public key to the repo.

Follows up, to do after committing this one.

  1. Sanitize the project configurations overall. I opened a new issue - #558
  2. Be able to build test projects in a signed version as well
    Follow Jon's comment:
    "The most likely reason for singing a test assembly is so that you can trust it with InternalsVisibleTo in the production code. I've lost where we've got to with friend assemblies, but it can certainly be very helpful. It means you may need conditional compilation, however - see https://github.com/nodatime/nodatime/blob/master/src/NodaTime/Properties/AssemblyInfo.cs"
    #558
peleyal added 3 commits Jun 11, 2015
Add a signing rule that uses the key from c:\code\google.apis.snk
- Change the release process and the publisher to sign dlls
- Add a git repository which is used by the release process.
@jskeet

This comment has been minimized.

Is this change meant to be part of the commit? It's not clear what it has to do with the release process.

This comment has been minimized.

Copy link

LindaLawton replied Jun 15, 2015

Originally we thought we would have to remove the internal stuff to get the projects signed. This was a mistake i think he is putting them back. Would it not have been better to just take a fresh copy of master and apply signing to it.

This comment has been minimized.

Copy link
Owner Author

peleyal replied Jun 15, 2015

I'm just adding back internal to methods that should be internal.
I think it's OK to use the head, since the change itself - c5019b1 was good , except this part.

Back in the time, Linda and I thought that we should remove all "friend" assemblies, but it's NOT required anymore.

@jskeet

This comment has been minimized.

Why doesn't this include AssemblyOriginatorKeyFile?

This comment has been minimized.

Copy link

LindaLawton replied Jun 15, 2015

Because I thought if we where using the key file it wasn't needed. This is my first expernce with DelaySigning, the documentation is a bit cryptic IMO. "If your project creates a COM wrapper assembly, you can use the AssemblyOriginatorKeyFile, AssemblyKeyContainerName, and DelaySign properties to sign the assembly. If both the AssemblyOriginatorKeyFile property and AssemblyKeyContainerName property exist, and the container name is found in the CSP, then the assembly is signed using the container. Otherwise, the key file is used to sign the assembly."

This comment has been minimized.

Copy link
Owner Author

peleyal replied Jun 15, 2015

I thought to NOT add the key here, since it's a testing project and not release!
If you strongly feel that we should let me know. I don't see why it should be important.

This comment has been minimized.

Copy link

jskeet replied Jun 30, 2015

(Sorry, didn't see this reply before.)

The most likely reason for singing a test assembly is so that you can trust it with InternalsVisibleTo in the production code. I've lost where we've got to with friend assemblies, but it can certainly be very helpful. It means you may need conditional compilation, however - see https://github.com/nodatime/nodatime/blob/master/src/NodaTime/Properties/AssemblyInfo.cs for an example of this.

@jskeet

This comment has been minimized.

What's the purpose of this, compared with ReleaseSigned|AnyCPU? They look the same to me.

This comment has been minimized.

Copy link
Owner Author

peleyal replied Jun 15, 2015

That's what VS created for me when I targeted "Release" at the first place, and now when I duplicate the configuration I got it.

@jskeet

This comment has been minimized.

Do we need an x86 build? (The platform target is still AnyCPU anyway, so it's not clear what this means...)

This comment has been minimized.

Copy link
Owner Author

peleyal replied Jun 15, 2015

I just duplicated the "Release" configuration. We should target "AnyCPU"

@jskeet

This comment has been minimized.

Same comments as above :)

@jskeet

This comment has been minimized.

This time we do have genuine ARM, x64 and x86 targets - but do we need them?

This comment has been minimized.

Copy link
Owner Author

peleyal replied Jun 15, 2015

Do you think that I should just clean all not "AnyCpu" configurations?

This comment has been minimized.

Copy link

jskeet replied Jun 30, 2015

Yes. It might be worth doing that in a different PR though - maybe get this one in, and then have a "tidy up" PR.

@jskeet

This comment has been minimized.

Copy link

jskeet commented on 4667e16 Jun 15, 2015

I haven't looked at all the projects we have for the client libraries, but it looks like we're creating more new configurations than we really need.

This comment has been minimized.

Copy link
Owner Author

peleyal replied Jun 15, 2015

I'll cc you on the configuration for the generated libraries internally.
It's pretty small since it's only PCL

This comment has been minimized.

Copy link

jskeet replied Jun 30, 2015

Okay, I think I'm happy with this if there's a follow-up PR to sanitize the project configurations overall - in my experience it can be somewhat painful to do, but the benefits are worthwhile in the longer term.

@jtattermusch

This comment has been minimized.

Copy link
Collaborator

jtattermusch commented Jun 16, 2015

Generally LGTM, I'd like to learn more about how we manage the .snk key (let's chat offline).

@@ -55,8 +55,8 @@
<Compile Include="Program.cs" />
<Compile Include="ProjectExtenstions.cs" />
<Compile Include="Properties\AssemblyInfo.cs" />
<Compile Include="Repositories\Git.cs" />
<Compile Include="Repositories\Hg.cs" />

This comment has been minimized.

Copy link
@LindaLawton

LindaLawton Jun 17, 2015

Collaborator

should hg.cs be removed?

This comment has been minimized.

Copy link
@peleyal

peleyal Jun 17, 2015

Author Collaborator

I think I would leave it for now, no harm... I know that github might work with Hg in the future, so why not include it as well...

@LindaLawton

This comment has been minimized.

Copy link
Collaborator

LindaLawton commented Jun 17, 2015

Should we remove the .hgignore file? I don't think we need it anymore do we.

peleyal added 3 commits Jun 26, 2015
Last commit was missing some right dependencies (usage of Zlib.Portable
instead Zlib.Portable.Signed NuGet package, dll name, by the way,
remained Zlib.Portable)
peleyal added a commit that referenced this pull request Jul 7, 2015
Issue #238 - Signed Google.Apis
@peleyal peleyal merged commit 02ddc01 into googleapis:master Jul 7, 2015
aiuto added a commit to google/apis-client-generator that referenced this pull request Nov 19, 2015
1. Fixed the string transformation where if a reserved keyword is used with
invalid characters, we were not stripping out the invalid character when
appending the reserved keyword with "__". This was causing something
like '$object' to be transformed into '$object__' instead of 'object__'

2. Support Strong Named Assemblies in C#
.NET Issue 238
(googleapis/google-api-dotnet-client#238) -
Support Strong Named Assemblies

Based on committing
googleapis/google-api-dotnet-client#550 (was pushed
on July 7th 2015)

3. Release 1.9.2 of the .NET client library for Google Apis.
- Clone tempalte directory for 1.9.2
- Add 1.9.2 as test to targets.json
- Clone golden tests for 1.9.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can鈥檛 perform that action at this time.