[PATCH] flashrom: Make packagers happy

[Carl-Daniel Hailfinger]

This patch introduces two new targets which are designed to make the
life of packagers easier.
In particular, it should no longer be necessary to patch the makefile
for hassle-free compilation.

The two new targets are:
make export
make tarball
Both preserve svn revisions and the exported tree does not depend on
subversion in any way or shape.

Documentation for this feature has been added to README.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-packaging_export/Makefile
===================================================================
--- flashrom-packaging_export/Makefile	(Revision 505)
+++ flashrom-packaging_export/Makefile	(Arbeitskopie)
@@ -41,9 +41,15 @@

# Set the flashrom version string from the highest revision number
 # of the checked out flashrom files.
-SVNDEF := -D'FLASHROM_VERSION="0.9.0-r$(shell svnversion -cn . \
-          | sed -e "s/.*://" -e "s/([0-9]*).*/\1/")"'
+# Note to packagers: Any tree exported with "make export" or "make tarball"
+# will not require any working subversion command.
+SVNVERSION := $(shell (LANG=C svnversion -cn . | grep -v exported || cat .svnversion) \
+          | sed -e "s/.*://" -e "s/([0-9]*).*/\1/")

+VERSION := 0.9.0-r$(SVNVERSION)
+
+SVNDEF := -D'FLASHROM_VERSION="$(VERSION)"'
+
 $(PROGRAM): $(OBJS)
    $(CC) -o $(PROGRAM) $(OBJS) $(LDFLAGS)
    $(STRIP) $(STRIP_ARGS) $(PROGRAM)
@@ -78,6 +84,15 @@
    mkdir -p $(PREFIX)/share/man/man8
    $(INSTALL) $(PROGRAM).8 $(PREFIX)/share/man/man8

+export:
+	@svn export . flashrom-$(VERSION)
+	@echo -n $(SVNVERSION) >flashrom-$(VERSION)/.svnversion
+	@echo Exported flashrom repository contents to flashrom-$(VERSION)/
+
+tarball: export
+	@tar cfz flashrom-$(VERSION).tar.gz flashrom-$(VERSION)/
+	@echo Created tarball of repository contents at flashrom-$(VERSION).tar.gz
+
 .PHONY: all clean distclean dep pciutils

-include .dependencies
Index: flashrom-packaging_export/README
===================================================================
--- flashrom-packaging_export/README	(Revision 505)
+++ flashrom-packaging_export/README	(Arbeitskopie)
@@ -12,6 +12,21 @@
 (see http://coreboot.org for details on coreboot)

+Packaging
+---------
+
+To package flashrom and remove dependencies on subversion, either use
+make export
+or
+make tarball
+
+make export will export all flashrom files from the subversion repository into
+a directory named flashrom-$VERSION-r$SVNREVISION and will additionally create
+a file .svnversion in that directory which stores the svn revision of the tree.
+
+make tarball will simply tar up the result of make export and gzip compress it.
+
+
 Build Requirements
 ------------------

[Peter Stuge]

Carl-Daniel Hailfinger wrote:
> @echo -n $(SVNVERSION) >flashrom-$(VERSION)/.svnversion

That's pretty ugly.

//Peter

[Carl-Daniel Hailfinger]

On 14.05.2009 04:23, Peter Stuge wrote:
> Carl-Daniel Hailfinger wrote:
> > @echo -n $(SVNVERSION) >flashrom-$(VERSION)/.svnversion
>
> That's pretty ugly.

It's the only way I saw to preserve the svn revision in the tarball
(well except $Rev$ which would have been sprinkled over every file to
get a reasonable result) and have identical makefile functionality both
for exported trees and svn trees.

The big problem I tried to address in this patch is that most
distributions patch out the part of the makefile which tries to retrieve
the svn revision because the tarball they ship has no .svn directories
any more. That way, we lose revision information for most distribution
packages.

Any hints on how to fix this in a different way are appreciated.

Regards,
Carl-Daniel

[Carl-Daniel Hailfinger]

On 14.05.2009 09:27, Carl-Daniel Hailfinger wrote:
> On 14.05.2009 04:23, Peter Stuge wrote:
> > Carl-Daniel Hailfinger wrote:
> > > @echo -n $(SVNVERSION) >flashrom-$(VERSION)/.svnversion
> >
> > That's pretty ugly.
>
> It's the only way I saw to preserve the svn revision in the tarball
> (well except $Rev$ which would have been sprinkled over every file to
> get a reasonable result) and have identical makefile functionality both
> for exported trees and svn trees.
>
> The big problem I tried to address in this patch is that most
> distributions patch out the part of the makefile which tries to retrieve
> the svn revision because the tarball they ship has no .svn directories
> any more. That way, we lose revision information for most distribution
> packages.
>
> Any hints on how to fix this in a different way are appreciated.

Ping?
Can the patch be applied even though it is ugly? Should I explain more?

Regards,
Carl-Daniel

[Peter Stuge]

Carl-Daniel Hailfinger wrote:
> > > > @echo -n $(SVNVERSION) >flashrom-$(VERSION)/.svnversion
> > >
> > > That's pretty ugly.
> >
> > It's the only way I saw to preserve the svn revision in the
> > tarball

We discussed this for the 0.9.0 release and you did the commits.
> > Any hints on how to fix this in a different way are appreciated.
>
> Can the patch be applied even though it is ugly?

That is a definite no.

When creating a tarball one simply changes SVNDEF in Makefile to a
string with the appropriate tarball version.

No tricks! And no ugly hidden files for no reason. KISS.

//Peter

[Carl-Daniel Hailfinger]

On 28.05.2009 06:34, Peter Stuge wrote:
> Carl-Daniel Hailfinger wrote:
> > > > > @echo -n $(SVNVERSION) >flashrom-$(VERSION)/.svnversion
> > > >
> > > > That's pretty ugly.
> > >
> > > It's the only way I saw to preserve the svn revision in the
> > > tarball
>
> We discussed this for the 0.9.0 release and you did the commits.

Right. That way works fine for releases, but not for snapshots which are
shipped by quite a lot of distributions (Debian, Gentoo, ...)
> > > Any hints on how to fix this in a different way are appreciated.
> >
> > Can the patch be applied even though it is ugly?
>
> That is a definite no.
>
> When creating a tarball one simply changes SVNDEF in Makefile to a
> string with the appropriate tarball version.

That works fine for me. Would you object to a patch which changes SVNDEF
in the Makefile during export? AFAICS that would address your objections
and still make packaging easier.
> No tricks! And no ugly hidden files for no reason. KISS.

OK.

Regards,
Carl-Daniel

[Peter Stuge]

Carl-Daniel Hailfinger wrote:
> > > > > > @echo -n $(SVNVERSION) >flashrom-$(VERSION)/.svnversion
> > > > >
> > > > > That's pretty ugly.
> > > >
> > > > It's the only way I saw to preserve the svn revision in the
> > > > tarball
> >
> > We discussed this for the 0.9.0 release and you did the commits.
>
> Right. That way works fine for releases, but not for snapshots
> which are shipped by quite a lot of distributions (Debian, Gentoo,
> ...)

The issue is that we do not create the snapshot tarballs. We should,
and part of that process would be to replace SVNDEF in Makefile.

I didn't quite understand what patch you propose though. :\

//Peter

[Carl-Daniel Hailfinger]

On 29.05.2009 15:30, Peter Stuge wrote:
> The issue is that we do not create the snapshot tarballs. We should,
> and part of that process would be to replace SVNDEF in Makefile.

We do provide snapshots of all repositories, but they are scattered all
over the place and I can't remember where flashrom ends up. I agree that
we need to have a page which points to all snapshots.
> I didn't quite understand what patch you propose though. :\

This patch introduces two new targets which are designed to make the
life of packagers easier.
In particular, it should no longer be necessary to patch the makefile
for hassle-free compilation.

The two new targets are:
make export
make tarball
Both preserve svn revisions and the exported tree does not depend on
subversion in any way or shape.

Documentation for this feature has been added to README.

Thanks to Peter Stuge for the suggestion to fix SVNDEF in the Makefile.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-packaging_export/Makefile
===================================================================
--- flashrom-packaging_export/Makefile	(Revision 555)
+++ flashrom-packaging_export/Makefile	(Arbeitskopie)
@@ -40,9 +40,14 @@

# Set the flashrom version string from the highest revision number
 # of the checked out flashrom files.
-SVNDEF := -D'FLASHROM_VERSION="0.9.0-r$(shell svnversion -cn . \
-          | sed -e "s/.*://" -e "s/([0-9]*).*/\1/")"'
+# Note to packagers: Any tree exported with "make export" or "make tarball"
+# will not require subversion.
+SVNVERSION := $(shell LANG=C svnversion -cn . | sed -e "s/.*://" -e "s/([0-9]*).*/\1/" | grep "[0-9]" || echo unknown)

+VERSION := 0.9.0-r$(SVNVERSION)
+
+SVNDEF := -D'FLASHROM_VERSION="$(VERSION)"'
+
 $(PROGRAM): $(OBJS)
    $(CC) $(LDFLAGS) -o $(PROGRAM) $(OBJS) $(LIBS)

@@ -80,6 +85,15 @@
    $(INSTALL) -m 0755 $(PROGRAM) $(DESTDIR)$(PREFIX)/sbin
    $(INSTALL) -m 0644 $(PROGRAM).8 $(DESTDIR)$(MANDIR)/man8

+export:
+	@svn export . flashrom-$(VERSION)
+	@sed "s/^SVNVERSION.*/SVNVERSION := $(SVNVERSION)/" Makefile >flashrom-$(VERSION)/Makefile
+	@echo Exported flashrom repository contents to flashrom-$(VERSION)/
+
+tarball: export
+	@tar cfz flashrom-$(VERSION).tar.gz flashrom-$(VERSION)/
+	@echo Created tarball of repository contents at flashrom-$(VERSION).tar.gz
+
 .PHONY: all clean distclean dep pciutils

-include .dependencies
Index: flashrom-packaging_export/README
===================================================================
--- flashrom-packaging_export/README	(Revision 555)
+++ flashrom-packaging_export/README	(Arbeitskopie)
@@ -15,6 +15,21 @@
 (see http://coreboot.org for details on coreboot)

+Packaging
+---------
+
+To package flashrom and remove dependencies on subversion, either use
+make export
+or
+make tarball
+
+make export will export all flashrom files from the subversion repository into
+a directory named flashrom-$VERSION-r$SVNREVISION and will additionally modify
+Makefile in that directory to contain the svn revision of the tree.
+
+make tarball will simply tar up the result of make export and gzip compress it.
+
+
 Build Instructions
 ------------------

[Peter Stuge]

Carl-Daniel Hailfinger wrote:
> > The issue is that we do not create the snapshot tarballs. We should,
> > and part of that process would be to replace SVNDEF in Makefile.
>
> We do provide snapshots of all repositories, but they are scattered
> all over the place and I can't remember where flashrom ends up.

I only find v2 and v3 at http://qa.coreboot.org/snapshots/
> I agree that we need to have a page which points to all snapshots.

Either on the Download page or on each relevant page. In the Flashrom
case I think it belongs on the Flashrom page.
> +# Note to packagers: Any tree exported with "make export" or "make tarball"
> +# will not require subversion.

Do we need to say so? It think it is really intuitive that a snapshot
doesn't require svn.
> +export:
> @svn export . flashrom-$(VERSION)
> @sed "s/^SVNVERSION.*/SVNVERSION := $(SVNVERSION)/" Makefile >flashrom-$(VERSION)/Makefile
> @echo Exported flashrom repository contents to flashrom-$(VERSION)/

Well, it exports the working copy, which might have changes. At least
add -r to the export. It would be very nice to make the rule also
apply to releases while at it. Right now it adds the rev back in. I
don't know how to do that very neatly however. :\
> +tarball: export
> @tar cfz flashrom-$(VERSION).tar.gz flashrom-$(VERSION)/
> @echo Created tarball of repository contents at flashrom-$(VERSION).tar.gz

Again, it's not really the repo contents. With -r it is, then maybe
just say Created $filename because tar and the rev are part of the
filename?
> .PHONY: all clean distclean dep pciutils

I think export and tarball should be added to .PHONY.
> +Packaging
> +---------

Really in the README? Do we have INSTALL? Maybe it's time?

//Peter

[Carl-Daniel Hailfinger]

On 29.05.2009 16:12, Peter Stuge wrote:
> Carl-Daniel Hailfinger wrote:
> > I can't remember where flashrom ends up.
>
> I only find v2 and v3 at http://qa.coreboot.org/snapshots/

I think it was part of a trac page. Anyway, a canonical download
location for flashrom is the way to go.
> > I agree that we need to have a page which points to all snapshots.
>
> Either on the Download page or on each relevant page. In the Flashrom
> case I think it belongs on the Flashrom page.

Agreed.
> > +# Note to packagers: Any tree exported with "make export" or "make tarball"
> > +# will not require subversion.
>
> Do we need to say so? It think it is really intuitive that a snapshot
> doesn't require svn.

Considering that packagers patch it out at the moment, I'd like to give
them a hint that make export solves the issue.
> > +export:
> > @svn export . flashrom-$(VERSION)
> > @sed "s/^SVNVERSION.*/SVNVERSION := $(SVNVERSION)/" Makefile >flashrom-$(VERSION)/Makefile
> > @echo Exported flashrom repository contents to flashrom-$(VERSION)/
>
> Well, it exports the working copy, which might have changes. At least
> add -r to the export. It would be very nice to make the rule also
> apply to releases while at it. Right now it adds the rev back in. I
> don't know how to do that very neatly however. :\

Originally, I wanted to have releases without svn revision. However, I
noticed that distributions tend to package newer snapshots under the
name of the release.
Having the revision in releases+snapshots also allows us to quickly go
to the correct revision without having to look it up or switch branches.
Plus, it kills two unnecessary commits per release.
> > +tarball: export
> > @tar cfz flashrom-$(VERSION).tar.gz flashrom-$(VERSION)/
> > @echo Created tarball of repository contents at flashrom-$(VERSION).tar.gz
>
> Again, it's not really the repo contents. With -r it is, then maybe
> just say Created $filename because tar and the rev are part of the
> filename?

OK. Should we use the (unmodified) repo contents or the directory contents?
> > .PHONY: all clean distclean dep pciutils
>
> I think export and tarball should be added to .PHONY.

Will do.
> > +Packaging
> > +---------
>
> Really in the README? Do we have INSTALL? Maybe it's time?

Good point. I'd postpone splitting to another patch, though.

I'll wait for your answer on the points above, then post a new patch. OK
for you?

Regards,
Carl-Daniel

[Peter Stuge]

Carl-Daniel Hailfinger wrote:
> I think it was part of a trac page.

Oh - yeah, I guess it's possible to get tarballs out of trac.
> Anyway, a canonical download location for flashrom is the way to
> go.

Yes for sure! It would be nice to have a flashrom directory in
qa/snapshots/

The posthook will be simple with these Makefile changes.
> > > +# Note to packagers: Any tree exported with "make export" or "make tarball"
> > > +# will not require subversion.
> >
> > Do we need to say so? It think it is really intuitive that a snapshot
> > doesn't require svn.
>
> Considering that packagers patch it out at the moment, I'd like to
> give them a hint that make export solves the issue.

Or we can provide tarballs. Everyone likes tarballs.
> Having the revision in releases+snapshots also allows us to quickly
> go to the correct revision without having to look it up or switch
> branches.

Sure.
> Plus, it kills two unnecessary commits per release.

Only unneccessary if they feel that way.
> > Well, it exports the working copy, which might have changes. At least
> > add -r to the export.
> > Again, it's not really the repo contents. With -r it is, then maybe
> > just say Created $filename because tar and the rev are part of the
> > filename?
>
> OK. Should we use the (unmodified) repo contents or the directory
> contents?

svn export -r from a path gives both. See --help

If the unique atom in the export is the rev number, I think it makes
sense to try to avoid changes made after that rev.
> > Really in the README? Do we have INSTALL? Maybe it's time?
>
> Good point. I'd postpone splitting to another patch, though.

I have no strong opinion.

//Peter

[Carl-Daniel Hailfinger]

Next try. [commit message snipped]

In addition to the changes requested by Peter, I also added support for
EXPORTDIR to avoid polluting the flashrom directory.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-packaging_export/Makefile
===================================================================
--- flashrom-packaging_export/Makefile	(Revision 555)
+++ flashrom-packaging_export/Makefile	(Arbeitskopie)
@@ -12,6 +12,7 @@
 PREFIX  ?= /usr/local
 MANDIR  ?= $(PREFIX)/share/man
 CFLAGS  ?= -Os -Wall -Werror
+EXPORTDIR ?= .

OS_ARCH	= $(shell uname)
 ifneq ($(OS_ARCH), SunOS)
@@ -40,9 +41,14 @@

# Set the flashrom version string from the highest revision number
 # of the checked out flashrom files.
-SVNDEF := -D'FLASHROM_VERSION="0.9.0-r$(shell svnversion -cn . \
-          | sed -e "s/.*://" -e "s/([0-9]*).*/\1/")"'
+# Note to packagers: Any tree exported with "make export" or "make tarball"
+# will not require subversion. The downloadable snapshots are already exported.
+SVNVERSION := $(shell LANG=C svnversion -cn . | sed -e "s/.*://" -e "s/([0-9]*).*/\1/" | grep "[0-9]" || echo unknown)

+VERSION := 0.9.0-r$(SVNVERSION)
+
+SVNDEF := -D'FLASHROM_VERSION="$(VERSION)"'
+
 $(PROGRAM): $(OBJS)
    $(CC) $(LDFLAGS) -o $(PROGRAM) $(OBJS) $(LIBS)

@@ -80,6 +86,17 @@
    $(INSTALL) -m 0755 $(PROGRAM) $(DESTDIR)$(PREFIX)/sbin
    $(INSTALL) -m 0644 $(PROGRAM).8 $(DESTDIR)$(MANDIR)/man8

-.PHONY: all clean distclean dep pciutils
+export:
+	@rm -rf $(EXPORTDIR)/flashrom-$(VERSION)
+	@svn export -r BASE . $(EXPORTDIR)/flashrom-$(VERSION)
+	@sed "s/^SVNVERSION.*/SVNVERSION := $(SVNVERSION)/" Makefile >$(EXPORTDIR)/flashrom-$(VERSION)/Makefile
+	@echo Exported $(EXPORTDIR)/flashrom-$(VERSION)/

+tarball: export
+	@tar cfz $(EXPORTDIR)/flashrom-$(VERSION).tar.gz -C $(EXPORTDIR)/ flashrom-$(VERSION)/
+	@rm -rf $(EXPORTDIR)/flashrom-$(VERSION)
+	@echo Created $(EXPORTDIR)/flashrom-$(VERSION).tar.gz
+
+.PHONY: all clean distclean dep pciutils export tarball
+
 -include .dependencies
Index: flashrom-packaging_export/README
===================================================================
--- flashrom-packaging_export/README	(Revision 555)
+++ flashrom-packaging_export/README	(Arbeitskopie)
@@ -15,6 +15,25 @@
 (see http://coreboot.org for details on coreboot)

+Packaging
+---------
+
+To package flashrom and remove dependencies on subversion, either use
+make export
+or
+make tarball
+
+make export will export all flashrom files from the subversion repository at
+revision BASE into a directory named $EXPORTDIR/flashrom-$VERSION-r$SVNREVISION
+and will additionally modify the Makefile in that directory to contain the svn
+revision of the exported tree.
+
+make tarball will simply tar up the result of make export and gzip compress it.
+
+The snapshot tarballs are the result of make tarball and require no further
+processing.
+
+
 Build Instructions
 ------------------

[Carl-Daniel Hailfinger]

On 30.05.2009 16:30, Carl-Daniel Hailfinger wrote:
> Two new targets:
> make export
> make tarball
> Both preserve svn revisions and the exported tree does not depend on
> subversion in any way or shape.
>
> Signed-off-by: Carl-Daniel Hailfinger

We need this for five reasons:
1. Packagers currently have to patch flashrom source to compile it on
systems without subversion. We should make it easier for them.
2. Snapshot tarballs currently have a .svn 1.5 directory included but
this will cause errors for users with older svn 1.4. Not requiring
subversion for snapshot compilation is best.
3. Since packagers seldom the svn revision in their fixup patches, some
packages out there have incorrect or no revision, only major version
numbers.
4. Releasing a new version of flashrom needs too many changes to the
makefile which have to be reverted instantly after the release. That is
unnecessary churn.
5. Making a release is easy with the change. Update the major version,
then run "make tarball".

Regards,
Carl-Daniel

[Carl-Daniel Hailfinger]

Ping.

Regards,
Carl-Daniel

On 05.06.2009 21:28, Carl-Daniel Hailfinger wrote:
> On 30.05.2009 16:30, Carl-Daniel Hailfinger wrote:
> > Two new targets:
> > make export
> > make tarball
> > Both preserve svn revisions and the exported tree does not depend on
> > subversion in any way or shape.
> >
> > Signed-off-by: Carl-Daniel Hailfinger
>
> We need this for five reasons:
> Packagers currently have to patch flashrom source to compile it on
>
> systems without subversion. We should make it easier for them.
> 2. Snapshot tarballs currently have a .svn 1.5 directory included but
> this will cause errors for users with older svn 1.4. Not requiring
> subversion for snapshot compilation is best.
> 3. Since packagers seldom the svn revision in their fixup patches, some
> packages out there have incorrect or no revision, only major version
> numbers.
> 4. Releasing a new version of flashrom needs too many changes to the
> makefile which have to be reverted instantly after the release. That is
> unnecessary churn.
> 5. Making a release is easy with the change. Update the major version,
> then run "make tarball".
>
> Regards,
> Carl-Daniel

[Uwe Hermann]

On Sat, May 30, 2009 at 04:30:04PM +0200, Carl-Daniel Hailfinger wrote:
> Signed-off-by: Carl-Daniel Hailfinger

Looks good to me, I also did a quick test and it works fine.

Acked-by: Uwe Hermann

Uwe.

[Carl-Daniel Hailfinger]

On 12.06.2009 16:15, Uwe Hermann wrote:
> On Sat, May 30, 2009 at 04:30:04PM +0200, Carl-Daniel Hailfinger wrote:
> > Signed-off-by: Carl-Daniel Hailfinger
>
> Looks good to me, I also did a quick test and it works fine.
>
> Acked-by: Uwe Hermann

Thanks, committed in r587.

Regards,
Carl-Daniel