Merge lp:~dobey/libubuntuone/srcdir-signing into lp:libubuntuone

Proposed by dobey
Status: Merged
Approved by: dobey
Approved revision: 113
Merged at revision: 112
Proposed branch: lp:~dobey/libubuntuone/srcdir-signing
Merge into: lp:libubuntuone
Diff against target: 92 lines (+25/-13)
4 files modified
.bzrignore (+1/-1)
bindings/mono/AssemblyInfo.cs.in (+1/-1)
bindings/mono/Makefile.am (+19/-11)
configure.ac (+4/-0)
To merge this branch: bzr merge lp:~dobey/libubuntuone/srcdir-signing
Reviewer Review Type Date Requested Status
Alejandro J. Cura (community) Approve
Natalia Bidart (community) Approve
Review via email: mp+43668@code.launchpad.net

Commit message

Generate the AssemblyInfo.cs file by replacing @ASSEMBLY_KEYFILE@
Require the sn program and use it to check the signature on the built dll

To post a comment you must log in.
Revision history for this message
Loïc Minier (lool) wrote :

That seems fine; some minor comments:
 * you might want to call sn under LC_ALL=C in case the output is localized at some point -- or test exit code if that's an option, not sure it reports this in the exit code though
 * you might want to send errors to stderr
 * instead of sn | sed + test, you could just grep? e.g.:
if LC_ALL=C $(MONO_SN) -T $< | grep -q '^Public Key Token: '; then \
   echo "Assembly was not properly signed." >&2; \
   exit 1; \
fi

 * I guess it's a personal taste question, but I think sed s/[@]FOO[@]/bar/ is slightly more common in Makefile.am than s/\@FOO\@/bar/

 * the Makefile dependency is weird:
+AssemblyInfo.cs: $(srcdir)/AssemblyInfo.cs.in $(srcdir)/mono.snk Makefile
 I don't think any other generated file depends on Makefile

 * I'm not actually sure why AssemblyInfo.cs depends on mono.snk; I think only the dll needs to do that

 * thanks to VPATH, I think you can actually just depend on the plain simple filenames:
AssemblyInfo.cs: AssemblyInfo.cs.in

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Code looks good, the branch builds correctly.

review: Approve
Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :

Voting does not meet specified criteria. Required: Approve >= 2, Disapprove == 0, Needs Fixing == 0, Needs Information == 0, Resubmit == 0. Got: 1 Approve.

Revision history for this message
dobey (dobey) wrote :

OK, not using $(srcdir)/ in the dependencies declarations seems to work, so I removed the instances where it was used. I also removed the |sed portion of the check rule, since we're not using it yet, but I figured it might be useful to perhaps use the key id in the future. It's easy to add back though and not calling sed saves us a microsecond of build time.

I made AssemblyInfo.cs depend on mono.snk because we're replacing a string with the path to it. If the source file doesn't exist, it should fail there, rather than wasting time generating the file and then trying to compile all the csharp files into the dll, only to have it fail later. Earlier failures are better.

As for depending on Makefile, anything which generates a file via automatic inclusion in another rule, like we do here, it should depend on Makefile so that the file gets rebuilt if Makefile changes. For instance, if any variables we're inserting into the file with sed like this, have changed, the file needs to be regenerated with the new values.

113. By dobey

Use sn -v to check the signature instead of parsing the -T output

Revision history for this message
dobey (dobey) wrote :

I've also discovered the -vf option now, and decided to use it instead of trying to parse the output. This keeps it language independent and relies on the return value instead.

Revision history for this message
Alejandro J. Cura (alecu) wrote :

Looks good

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file '.bzrignore'
--- .bzrignore 2010-08-25 18:35:07 +0000
+++ .bzrignore 2010-12-14 19:13:48 +0000
@@ -36,7 +36,7 @@
3636
37libubuntuone-*.tar.*37libubuntuone-*.tar.*
3838
39bindings/mono/[B-Za-z]*.cs39bindings/mono/*.cs
40bindings/mono/ubuntuone.sources40bindings/mono/ubuntuone.sources
41bindings/mono/ubuntuone-api.xml41bindings/mono/ubuntuone-api.xml
42bindings/python/ubuntuone.c42bindings/python/ubuntuone.c
4343
=== renamed file 'bindings/mono/AssemblyInfo.cs' => 'bindings/mono/AssemblyInfo.cs.in'
--- bindings/mono/AssemblyInfo.cs 2010-03-08 15:30:21 +0000
+++ bindings/mono/AssemblyInfo.cs.in 2010-12-14 19:13:48 +0000
@@ -8,4 +8,4 @@
8[assembly: AssemblyCopyright("Copyright 2010, Jo Shields")]8[assembly: AssemblyCopyright("Copyright 2010, Jo Shields")]
9[assembly: AssemblyDescription("Ubuntu One binding for Mono and .NET")]9[assembly: AssemblyDescription("Ubuntu One binding for Mono and .NET")]
10[assembly: AssemblyVersion("1.0.0.0")]10[assembly: AssemblyVersion("1.0.0.0")]
11[assembly: AssemblyKeyFile("mono.snk")]11[assembly: AssemblyKeyFile("@ASSEMBLY_KEYFILE@")]
1212
=== modified file 'bindings/mono/Makefile.am'
--- bindings/mono/Makefile.am 2010-08-25 18:35:07 +0000
+++ bindings/mono/Makefile.am 2010-12-14 19:13:48 +0000
@@ -1,31 +1,39 @@
1CLEANFILES = \1CLEANFILES = \
2 U1MusicStore.cs \2 *.cs \
3 OAuthMethod.cs \
4 ObjectManager.cs \
5 PlayLibraryHandler.cs \
6 PreviewMp3Handler.cs \
7 DownloadFinishedHandler.cs \
8 UrlLoadedHandler.cs \
9 ubuntuone.sources \3 ubuntuone.sources \
10 ubuntuone-sharp.dll \4 ubuntuone-sharp.dll \
11 ubuntuone-api.xml \5 ubuntuone-api.xml \
12 ubuntuone-sharp.dll.config6 ubuntuone-sharp.dll.config
137
14EXTRA_DIST = ubuntuone.sources.in ubuntuone-api.metadata mono.snk AssemblyInfo.cs8EXTRA_DIST = \
9 AssemblyInfo.cs.in \
10 mono.snk \
11 ubuntuone.sources.in \
12 ubuntuone-api.metadata
1513
16ubuntuonesharpdir = $(libdir)/mono/ubuntuone-sharp-1.014ubuntuonesharpdir = $(libdir)/mono/ubuntuone-sharp-1.0
17ubuntuonesharp_DATA = ubuntuone-sharp.dll ubuntuone-sharp.dll.config15ubuntuonesharp_DATA = ubuntuone-sharp.dll ubuntuone-sharp.dll.config
1816
19ubuntuone-sharp.dll: U1MusicStore.cs17check: ubuntuone-sharp.dll
18 @echo "Checking for valid signature."
19 if ! $(MONO_SN) -vf $<; then \
20 echo "Assembly was not properly signed."; \
21 exit 1; \
22 fi
23
24ubuntuone-sharp.dll: AssemblyInfo.cs U1MusicStore.cs
20 $(MONO_CSC) -target:library -pkg:gtk-sharp-2.0 -out:ubuntuone-sharp.dll *.cs25 $(MONO_CSC) -target:library -pkg:gtk-sharp-2.0 -out:ubuntuone-sharp.dll *.cs
2126
27AssemblyInfo.cs: AssemblyInfo.cs.in mono.snk Makefile
28 sed -e "s|\@ASSEMBLY_KEYFILE\@|$(srcdir)/mono.snk|g" < $< > $@
29
22U1MusicStore.cs: ubuntuone-api.xml30U1MusicStore.cs: ubuntuone-api.xml
23 $(MONO_GAPI_CODEGEN) --outdir=. $(GTKSHARP_CFLAGS) --generate $<31 $(MONO_GAPI_CODEGEN) --outdir=. $(GTKSHARP_CFLAGS) --generate $<
2432
25ubuntuone.sources: $(srcdir)/ubuntuone.sources.in33ubuntuone.sources: ubuntuone.sources.in Makefile
26 sed -e "s|\@TOP_SRCDIR\@|$(top_srcdir)|g" < $< > $@34 sed -e "s|\@TOP_SRCDIR\@|$(top_srcdir)|g" < $< > $@
2735
28ubuntuone-api.xml: ubuntuone.sources36ubuntuone-api.xml: ubuntuone.sources ubuntuone-api.metadata
29 $(MONO_GAPI_PARSER) $<37 $(MONO_GAPI_PARSER) $<
30 $(MONO_GAPI_FIXUP) --api=$@ --metadata=$(srcdir)/ubuntuone-api.metadata38 $(MONO_GAPI_FIXUP) --api=$@ --metadata=$(srcdir)/ubuntuone-api.metadata
3139
3240
=== modified file 'configure.ac'
--- configure.ac 2010-11-24 18:05:49 +0000
+++ configure.ac 2010-12-14 19:13:48 +0000
@@ -111,6 +111,10 @@
111 AC_MSG_ERROR(could not find gmcs compiler)111 AC_MSG_ERROR(could not find gmcs compiler)
112fi112fi
113113
114AC_PATH_PROG([MONO_SN], [sn], [no])
115if test "x$MONO_SN" = "xno"; then
116 AC_MSG_ERROR([could not find sn])
117fi
114118
115dnl Checks for Python bindings119dnl Checks for Python bindings
116AM_PATH_PYTHON(2.5)120AM_PATH_PYTHON(2.5)

Subscribers

People subscribed via source and target branches