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

[dobey]

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

[Loïc Minier (lool)]

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

[Natalia Bidart (nataliabidart)]

Code looks good, the branch builds correctly.

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

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

[dobey (dobey)]

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.

[lp:~dobey/libubuntuone/srcdir-signing]

113. By dobey on 2010-12-14

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

[dobey (dobey)]

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.

[Alejandro J. Cura (alecu)]

Looks good

review: Approve

=== modified file '.bzrignore'
--- .bzrignore	2010-08-25 18:35:07 +0000
+++ .bzrignore	2010-12-14 19:13:48 +0000
@@ -36,7 +36,7 @@
 
 libubuntuone-*.tar.*
 
-bindings/mono/[B-Za-z]*.cs
+bindings/mono/*.cs
 bindings/mono/ubuntuone.sources
 bindings/mono/ubuntuone-api.xml
 bindings/python/ubuntuone.c

=== 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 @@
 [assembly: AssemblyCopyright("Copyright 2010, Jo Shields")]
 [assembly: AssemblyDescription("Ubuntu One binding for Mono and .NET")]
 [assembly: AssemblyVersion("1.0.0.0")]
-[assembly: AssemblyKeyFile("mono.snk")]
+[assembly: AssemblyKeyFile("@ASSEMBLY_KEYFILE@")]

=== 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 @@
 CLEANFILES = 	\
-	U1MusicStore.cs \
-	OAuthMethod.cs \
-	ObjectManager.cs \
-	PlayLibraryHandler.cs \
-	PreviewMp3Handler.cs \
-	DownloadFinishedHandler.cs \
-	UrlLoadedHandler.cs \
+	*.cs \
 	ubuntuone.sources \
 	ubuntuone-sharp.dll \
 	ubuntuone-api.xml \
 	ubuntuone-sharp.dll.config
 
-EXTRA_DIST = ubuntuone.sources.in ubuntuone-api.metadata mono.snk AssemblyInfo.cs
+EXTRA_DIST = \
+	AssemblyInfo.cs.in \
+	mono.snk \
+	ubuntuone.sources.in \
+	ubuntuone-api.metadata
 
 ubuntuonesharpdir = $(libdir)/mono/ubuntuone-sharp-1.0
 ubuntuonesharp_DATA = ubuntuone-sharp.dll ubuntuone-sharp.dll.config
 
-ubuntuone-sharp.dll: U1MusicStore.cs
+check: ubuntuone-sharp.dll
+	@echo "Checking for valid signature."
+	if ! $(MONO_SN) -vf $<; then \
+		echo "Assembly was not properly signed."; \
+		exit 1; \
+	fi
+
+ubuntuone-sharp.dll: AssemblyInfo.cs U1MusicStore.cs
 	$(MONO_CSC) -target:library -pkg:gtk-sharp-2.0 -out:ubuntuone-sharp.dll *.cs
 
+AssemblyInfo.cs: AssemblyInfo.cs.in mono.snk Makefile
+	sed -e "s|\@ASSEMBLY_KEYFILE\@|$(srcdir)/mono.snk|g" < $< > $@
+
 U1MusicStore.cs: ubuntuone-api.xml
 	$(MONO_GAPI_CODEGEN) --outdir=. $(GTKSHARP_CFLAGS) --generate $<
 
-ubuntuone.sources: $(srcdir)/ubuntuone.sources.in
+ubuntuone.sources: ubuntuone.sources.in Makefile
 	sed -e "s|\@TOP_SRCDIR\@|$(top_srcdir)|g" < $< > $@
 
-ubuntuone-api.xml: ubuntuone.sources
+ubuntuone-api.xml: ubuntuone.sources ubuntuone-api.metadata
 	$(MONO_GAPI_PARSER) $<
 	$(MONO_GAPI_FIXUP) --api=$@ --metadata=$(srcdir)/ubuntuone-api.metadata
 

=== 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 @@
   AC_MSG_ERROR(could not find gmcs compiler)
 fi
 
+AC_PATH_PROG([MONO_SN], [sn], [no])
+if test "x$MONO_SN" = "xno"; then
+   AC_MSG_ERROR([could not find sn])
+fi
 
 dnl Checks for Python bindings
 AM_PATH_PYTHON(2.5)