| Submitter | Carl-Daniel Hailfinger |
|---|---|
| Date | 2010-08-21 18:21:41 |
| Message ID | <4C701935.9020008@gmx.net> |
| Download | mbox | patch |
| Permalink | /patch/1780/ |
| State | Superseded |
| Headers | show |
Comments
Looks pretty good so far! For the benefit of everyone who missed the excitement on IRC, there is one major caveat to this patch: Those using a git repository probably have stale SVN metadata, which means the upstream Flashrom SVN information in the version string will be incorrect. For example, if I clone the Chromium OS flashrom branch (git clone http://src.chromium.org/git/flashrom.git) and run "git svn info", I'll get a message saying "Unable to determine upstream SVN information from working tree history." After adding the upstream SVN repo as a remote repository and doing "git svn fetch", I'll get something like this: dhendrix@thegates:flashrom$ git svn info Path: . URL: svn://coreboot.org/flashrom/trunk Repository Root: svn://coreboot.org/flashrom Repository UUID: 2b7e53f0-3cfb-0310-b3e9-8179ed1497e1 Revision: 1011 Node Kind: directory Schedule: normal Last Changed Author: uwe Last Changed Rev: 1011 Last Changed Date: 2010-05-24 10:39:14 -0700 (Mon, 24 May 2010) In this example, r1011 is incorrect. Carl-Daniel root caused the issue and has a script that seems to workaround it. Here is his blogpost which summarizes the problem and workaround very well: http://blogs.coreboot.org/blog/2010/08/27/git-svn-info-unable-to-determine-upstream-svn-information-from-working-tree-history/ Using Carl-Daniel's script in the same repo, I get: dhendrix@thegates:flashrom$ sh /tmp/cdh.sh URL: svn://coreboot.org/flashrom/trunk Repository UUID: 2b7e53f0-3cfb-0310-b3e9-8179ed1497e1 Revision: 1145 Last Changed Author: hailfinger which is (currently) correct. Now to figure out how best to integrate this... Can we put the script in util/ and have the Makefile invoke it when it detects we're using a git repo? On Sat, Aug 21, 2010 at 11:21 AM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote: > flashrom relies on svn to export tarballs and only prints the last svn > revision in the version message even if flashrom is in a git tree. > > Print extended version information (version, svn revision, git hash, > repository URL). > Allow exporting from git trees. > > This should make git users first-class citizens like svn users. > > Side note: If anyone has an idea how to make the export target more > beautiful (80 column limit etc.), please tell me. > > Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> > > Index: flashrom-gitfriendly/Makefile > =================================================================== > --- flashrom-gitfriendly/Makefile (Revision 1145) > +++ flashrom-gitfriendly/Makefile (Arbeitskopie) > @@ -97,13 +97,15 @@ > # of the checked out flashrom files. > # Note to packagers: Any tree exported with "make export" or "make > tarball" > # will not require subversion. The downloadable snapshots are already > exported. > -SVNVERSION := $(shell LC_ALL=C svnversion -cn . 2>/dev/null | sed -e > "s/.*://" -e "s/\([0-9]*\).*/\1/" | grep "[0-9]" || LC_ALL=C svn info . > 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || LC_ALL=C git > svn info . 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || > echo unknown) > +SCMVERSION := $(shell LC_ALL=C svnversion -cn . 2>/dev/null | sed -e > "s/.*://" -e "s/\([0-9]*\).*/\1/" | grep "[0-9]" || LC_ALL=C svn info . > 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || LC_ALL=C git > svn info . 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || > echo unknown) > +SCMEXTVERSION := $(shell LC_ALL=C git rev-parse HEAD 2>/dev/null | sed > 's/^./-\0/') > +SCMURL := $(shell LC_ALL=C svn info 2>/dev/null | grep URL: | sed > 's/.*URL:[[:blank:]]*//' | grep ^. || LC_ALL=C git remote show origin > 2>/dev/null |grep URL: | sed 's/.*URL:[[:blank:]]*//' | grep ^. || echo > unknown ) > > RELEASE := 0.9.2 > -VERSION := $(RELEASE)-r$(SVNVERSION) > +VERSION := $(RELEASE)-r$(SCMVERSION)$(SCMEXTVERSION) > RELEASENAME ?= $(VERSION) > > -SVNDEF := -D'FLASHROM_VERSION="$(VERSION)"' > +SCMDEF := -D'FLASHROM_VERSION="$(VERSION)"' > -D'FLASHROM_SCMURL="$(SCMURL)"' > > # Always enable internal/onboard support for now. > CONFIG_INTERNAL ?= yes > @@ -310,7 +312,7 @@ > TAROPTIONS = $(shell LC_ALL=C tar --version|grep -q GNU && echo > "--owner=root --group=root") > > %.o: %.c .features > - $(CC) -MMD $(CFLAGS) $(CPPFLAGS) $(FEATURE_CFLAGS) $(SVNDEF) -o $@ > -c $< > + $(CC) -MMD $(CFLAGS) $(CPPFLAGS) $(FEATURE_CFLAGS) $(SCMDEF) -o $@ > -c $< > > # Make sure to add all names of generated binaries here. > # This includes all frontends and libflashrom. > @@ -420,9 +422,12 @@ > > export: > @rm -rf $(EXPORTDIR)/flashrom-$(RELEASENAME) > - @svn export -r BASE . $(EXPORTDIR)/flashrom-$(RELEASENAME) > - @sed "s/^SVNVERSION.*/SVNVERSION := $(SVNVERSION)/" Makefile > >$(EXPORTDIR)/flashrom-$(RELEASENAME)/Makefile > - @LC_ALL=C svn log >$(EXPORTDIR)/flashrom-$(RELEASENAME)/ChangeLog > + @svn export -r BASE . $(EXPORTDIR)/flashrom-$(RELEASENAME) > 2>/dev/null || git checkout-index -a -f > --prefix=$(EXPORTDIR)/flashrom-$(RELEASENAME)/ > + @# If SCMVERSION or SCMEXTVERSION contain a slash or SCMURL > contains a hash, this will explode > + @sed "s/^SCMVERSION.*/SCMVERSION := > $(SCMVERSION)/;s/^SCMEXTVERSION.*/SCMEXTVERSION := > $(SCMEXTVERSION)/;s#^SCMURL.*#SCMURL := $(SCMURL)#" Makefile > >$(EXPORTDIR)/flashrom-$(RELEASENAME)/Makefile > + @# ChangeLog should be in English, but we want UTF-8 for names. > + @( LC_ALL=en_US.UTF-8 svn log 2>/dev/null || LC_ALL=en_US.UTF-8 git > log 2>/dev/null || echo "Unable to extract log from SCM" ) > >$(EXPORTDIR)/flashrom-$(RELEASENAME)/ChangeLog > + @rm -f $(EXPORTDIR)/flashrom-$(RELEASENAME)/.gitignore > $(EXPORTDIR)/flashrom-$(RELEASENAME)/.gitattributes > $(EXPORTDIR)/flashrom-$(RELEASENAME)/.gitmodules > @echo Exported $(EXPORTDIR)/flashrom-$(RELEASENAME)/ > > tarball: export > Index: flashrom-gitfriendly/flashrom.c > =================================================================== > --- flashrom-gitfriendly/flashrom.c (Revision 1145) > +++ flashrom-gitfriendly/flashrom.c (Arbeitskopie) > @@ -1355,6 +1355,7 @@ > void print_version(void) > { > msg_ginfo("flashrom v%s", flashrom_version); > + msg_ginfo(" from " FLASHROM_SCMURL ","); > print_sysinfo(); > } > > @@ -1362,7 +1363,6 @@ > { > msg_ginfo("flashrom is free software, get the source code at " > "http://www.flashrom.org\n"); > - msg_ginfo("\n"); > } > > int selfcheck(void) > > > -- > http://www.hailfinger.org/ > > > _______________________________________________ > flashrom mailing list > flashrom@flashrom.org > http://www.flashrom.org/mailman/listinfo/flashrom >
[+vadim] On Fri, Aug 27, 2010 at 1:49 PM, David Hendricks <dhendrix@google.com>wrote: > Looks pretty good so far! > > For the benefit of everyone who missed the excitement on IRC, there is one > major caveat to this patch: Those using a git repository probably have stale > SVN metadata, which means the upstream Flashrom SVN information in the > version string will be incorrect. > > For example, if I clone the Chromium OS flashrom branch (git clone > http://src.chromium.org/git/flashrom.git) and run "git svn info", I'll get > a message saying "Unable to determine upstream SVN information from working > tree history." After adding the upstream SVN repo as a remote repository and > doing "git svn fetch", I'll get something like this: > dhendrix@thegates:flashrom$ git svn info > Path: . > URL: svn://coreboot.org/flashrom/trunk > Repository Root: svn://coreboot.org/flashrom > Repository UUID: 2b7e53f0-3cfb-0310-b3e9-8179ed1497e1 > Revision: 1011 > Node Kind: directory > Schedule: normal > Last Changed Author: uwe > Last Changed Rev: 1011 > Last Changed Date: 2010-05-24 10:39:14 -0700 (Mon, 24 May 2010) > > In this example, r1011 is incorrect. > > Carl-Daniel root caused the issue and has a script that seems to workaround > it. Here is his blogpost which summarizes the problem and workaround very > well: > http://blogs.coreboot.org/blog/2010/08/27/git-svn-info-unable-to-determine-upstream-svn-information-from-working-tree-history/ > > Using Carl-Daniel's script in the same repo, I get: > dhendrix@thegates:flashrom$ sh /tmp/cdh.sh > URL: svn://coreboot.org/flashrom/trunk > Repository UUID: 2b7e53f0-3cfb-0310-b3e9-8179ed1497e1 > Revision: 1145 > Last Changed Author: hailfinger > > which is (currently) correct. > > Now to figure out how best to integrate this... Can we put the script in > util/ and have the Makefile invoke it when it detects we're using a git > repo? > > On Sat, Aug 21, 2010 at 11:21 AM, Carl-Daniel Hailfinger < > c-d.hailfinger.devel.2006@gmx.net> wrote: > >> flashrom relies on svn to export tarballs and only prints the last svn >> revision in the version message even if flashrom is in a git tree. >> >> Print extended version information (version, svn revision, git hash, >> repository URL). >> Allow exporting from git trees. >> >> This should make git users first-class citizens like svn users. >> >> Side note: If anyone has an idea how to make the export target more >> beautiful (80 column limit etc.), please tell me. >> >> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> >> >> Index: flashrom-gitfriendly/Makefile >> =================================================================== >> --- flashrom-gitfriendly/Makefile (Revision 1145) >> +++ flashrom-gitfriendly/Makefile (Arbeitskopie) >> @@ -97,13 +97,15 @@ >> # of the checked out flashrom files. >> # Note to packagers: Any tree exported with "make export" or "make >> tarball" >> # will not require subversion. The downloadable snapshots are already >> exported. >> -SVNVERSION := $(shell LC_ALL=C svnversion -cn . 2>/dev/null | sed -e >> "s/.*://" -e "s/\([0-9]*\).*/\1/" | grep "[0-9]" || LC_ALL=C svn info . >> 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || LC_ALL=C git >> svn info . 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || >> echo unknown) >> +SCMVERSION := $(shell LC_ALL=C svnversion -cn . 2>/dev/null | sed -e >> "s/.*://" -e "s/\([0-9]*\).*/\1/" | grep "[0-9]" || LC_ALL=C svn info . >> 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || LC_ALL=C git >> svn info . 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || >> echo unknown) >> +SCMEXTVERSION := $(shell LC_ALL=C git rev-parse HEAD 2>/dev/null | sed >> 's/^./-\0/') >> +SCMURL := $(shell LC_ALL=C svn info 2>/dev/null | grep URL: | sed >> 's/.*URL:[[:blank:]]*//' | grep ^. || LC_ALL=C git remote show origin >> 2>/dev/null |grep URL: | sed 's/.*URL:[[:blank:]]*//' | grep ^. || echo >> unknown ) >> >> RELEASE := 0.9.2 >> -VERSION := $(RELEASE)-r$(SVNVERSION) >> +VERSION := $(RELEASE)-r$(SCMVERSION)$(SCMEXTVERSION) >> RELEASENAME ?= $(VERSION) >> >> -SVNDEF := -D'FLASHROM_VERSION="$(VERSION)"' >> +SCMDEF := -D'FLASHROM_VERSION="$(VERSION)"' >> -D'FLASHROM_SCMURL="$(SCMURL)"' >> >> # Always enable internal/onboard support for now. >> CONFIG_INTERNAL ?= yes >> @@ -310,7 +312,7 @@ >> TAROPTIONS = $(shell LC_ALL=C tar --version|grep -q GNU && echo >> "--owner=root --group=root") >> >> %.o: %.c .features >> - $(CC) -MMD $(CFLAGS) $(CPPFLAGS) $(FEATURE_CFLAGS) $(SVNDEF) -o $@ >> -c $< >> + $(CC) -MMD $(CFLAGS) $(CPPFLAGS) $(FEATURE_CFLAGS) $(SCMDEF) -o $@ >> -c $< >> >> # Make sure to add all names of generated binaries here. >> # This includes all frontends and libflashrom. >> @@ -420,9 +422,12 @@ >> >> export: >> @rm -rf $(EXPORTDIR)/flashrom-$(RELEASENAME) >> - @svn export -r BASE . $(EXPORTDIR)/flashrom-$(RELEASENAME) >> - @sed "s/^SVNVERSION.*/SVNVERSION := $(SVNVERSION)/" Makefile >> >$(EXPORTDIR)/flashrom-$(RELEASENAME)/Makefile >> - @LC_ALL=C svn log >$(EXPORTDIR)/flashrom-$(RELEASENAME)/ChangeLog >> + @svn export -r BASE . $(EXPORTDIR)/flashrom-$(RELEASENAME) >> 2>/dev/null || git checkout-index -a -f >> --prefix=$(EXPORTDIR)/flashrom-$(RELEASENAME)/ >> + @# If SCMVERSION or SCMEXTVERSION contain a slash or SCMURL >> contains a hash, this will explode >> + @sed "s/^SCMVERSION.*/SCMVERSION := >> $(SCMVERSION)/;s/^SCMEXTVERSION.*/SCMEXTVERSION := >> $(SCMEXTVERSION)/;s#^SCMURL.*#SCMURL := $(SCMURL)#" Makefile >> >$(EXPORTDIR)/flashrom-$(RELEASENAME)/Makefile >> + @# ChangeLog should be in English, but we want UTF-8 for names. >> + @( LC_ALL=en_US.UTF-8 svn log 2>/dev/null || LC_ALL=en_US.UTF-8 >> git log 2>/dev/null || echo "Unable to extract log from SCM" ) >> >$(EXPORTDIR)/flashrom-$(RELEASENAME)/ChangeLog >> + @rm -f $(EXPORTDIR)/flashrom-$(RELEASENAME)/.gitignore >> $(EXPORTDIR)/flashrom-$(RELEASENAME)/.gitattributes >> $(EXPORTDIR)/flashrom-$(RELEASENAME)/.gitmodules >> @echo Exported $(EXPORTDIR)/flashrom-$(RELEASENAME)/ >> >> tarball: export >> Index: flashrom-gitfriendly/flashrom.c >> =================================================================== >> --- flashrom-gitfriendly/flashrom.c (Revision 1145) >> +++ flashrom-gitfriendly/flashrom.c (Arbeitskopie) >> @@ -1355,6 +1355,7 @@ >> void print_version(void) >> { >> msg_ginfo("flashrom v%s", flashrom_version); >> + msg_ginfo(" from " FLASHROM_SCMURL ","); >> print_sysinfo(); >> } >> >> @@ -1362,7 +1363,6 @@ >> { >> msg_ginfo("flashrom is free software, get the source code at " >> "http://www.flashrom.org\n"); >> - msg_ginfo("\n"); >> } >> >> int selfcheck(void) >> >> >> -- >> http://www.hailfinger.org/ >> >> >> _______________________________________________ >> flashrom mailing list >> flashrom@flashrom.org >> http://www.flashrom.org/mailman/listinfo/flashrom >> > > > > -- > David Hendricks (dhendrix) > Systems Software Engineer, Google Inc. >
On Fri, Aug 27, 2010 at 1:52 PM, David Hendricks <dhendrix@google.com> wrote: > [+vadim] > > On Fri, Aug 27, 2010 at 1:49 PM, David Hendricks <dhendrix@google.com> > wrote: >> >> Looks pretty good so far! >> For the benefit of everyone who missed the excitement on IRC, there is one >> major caveat to this patch: Those using a git repository probably have stale >> SVN metadata, which means the upstream Flashrom SVN information in the >> version string will be incorrect. >> For example, if I clone the Chromium OS flashrom branch (git clone >> http://src.chromium.org/git/flashrom.git) and run "git svn info", I'll get a >> message saying "Unable to determine upstream SVN information from working >> tree history." After adding the upstream SVN repo as a remote repository and >> doing "git svn fetch", I'll get something like this: >> dhendrix@thegates:flashrom$ git svn info >> Path: . >> URL: svn://coreboot.org/flashrom/trunk >> Repository Root: svn://coreboot.org/flashrom >> Repository UUID: 2b7e53f0-3cfb-0310-b3e9-8179ed1497e1 >> Revision: 1011 >> Node Kind: directory >> Schedule: normal >> Last Changed Author: uwe >> Last Changed Rev: 1011 >> Last Changed Date: 2010-05-24 10:39:14 -0700 (Mon, 24 May 2010) >> In this example, r1011 is incorrect. >> Carl-Daniel root caused the issue and has a script that seems to >> workaround it. Here is his blogpost which summarizes the problem and >> workaround very >> well: http://blogs.coreboot.org/blog/2010/08/27/git-svn-info-unable-to-determine-upstream-svn-information-from-working-tree-history/ >> Using Carl-Daniel's script in the same repo, I get: >> dhendrix@thegates:flashrom$ sh /tmp/cdh.sh >> URL: svn://coreboot.org/flashrom/trunk >> Repository UUID: 2b7e53f0-3cfb-0310-b3e9-8179ed1497e1 >> Revision: 1145 >> Last Changed Author: hailfinger >> which is (currently) correct. >> Now to figure out how best to integrate this... Can we put the script in >> util/ and have the Makefile invoke it when it detects we're using a git >> repo? guys, I am joining the party late so to speak, I have a couple of questions: - why do you need to integrate shell scripts into the Makefile. It is perfectly valid to put the code in a file in ./util/ - when local git tree (or svn tree to that matter) has modified files - this should be reflected in the version string, to indicate that the code was built off modified source tree. - the git hash on its own is not good enough: you can't tell looking at two hashes which one is more recent, this is why in case svn information is not available the date should be included. In case the source is built off pristine sources, the date should be the git log date, in case the tree was modified, the date should be the build date. - *anything* isbetter than producing multiple different images with the same version string I have a patch which achieves most of that, I'll share it with you in a bit later, cheers, /vb - it is much better to have a stale svn revision >> On Sat, Aug 21, 2010 at 11:21 AM, Carl-Daniel Hailfinger >> <c-d.hailfinger.devel.2006@gmx.net> wrote: >>> >>> flashrom relies on svn to export tarballs and only prints the last svn >>> revision in the version message even if flashrom is in a git tree. >>> >>> Print extended version information (version, svn revision, git hash, >>> repository URL). >>> Allow exporting from git trees. >>> >>> This should make git users first-class citizens like svn users. >>> >>> Side note: If anyone has an idea how to make the export target more >>> beautiful (80 column limit etc.), please tell me. >>> >>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> >>> >>> Index: flashrom-gitfriendly/Makefile >>> =================================================================== >>> --- flashrom-gitfriendly/Makefile (Revision 1145) >>> +++ flashrom-gitfriendly/Makefile (Arbeitskopie) >>> @@ -97,13 +97,15 @@ >>> # of the checked out flashrom files. >>> # Note to packagers: Any tree exported with "make export" or "make >>> tarball" >>> # will not require subversion. The downloadable snapshots are already >>> exported. >>> -SVNVERSION := $(shell LC_ALL=C svnversion -cn . 2>/dev/null | sed -e >>> "s/.*://" -e "s/\([0-9]*\).*/\1/" | grep "[0-9]" || LC_ALL=C svn info . >>> 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || LC_ALL=C git >>> svn info . 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || >>> echo unknown) >>> +SCMVERSION := $(shell LC_ALL=C svnversion -cn . 2>/dev/null | sed -e >>> "s/.*://" -e "s/\([0-9]*\).*/\1/" | grep "[0-9]" || LC_ALL=C svn info . >>> 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || LC_ALL=C git >>> svn info . 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || >>> echo unknown) >>> +SCMEXTVERSION := $(shell LC_ALL=C git rev-parse HEAD 2>/dev/null | sed >>> 's/^./-\0/') >>> +SCMURL := $(shell LC_ALL=C svn info 2>/dev/null | grep URL: | sed >>> 's/.*URL:[[:blank:]]*//' | grep ^. || LC_ALL=C git remote show origin >>> 2>/dev/null |grep URL: | sed 's/.*URL:[[:blank:]]*//' | grep ^. || echo >>> unknown ) >>> >>> RELEASE := 0.9.2 >>> -VERSION := $(RELEASE)-r$(SVNVERSION) >>> +VERSION := $(RELEASE)-r$(SCMVERSION)$(SCMEXTVERSION) >>> RELEASENAME ?= $(VERSION) >>> >>> -SVNDEF := -D'FLASHROM_VERSION="$(VERSION)"' >>> +SCMDEF := -D'FLASHROM_VERSION="$(VERSION)"' >>> -D'FLASHROM_SCMURL="$(SCMURL)"' >>> >>> # Always enable internal/onboard support for now. >>> CONFIG_INTERNAL ?= yes >>> @@ -310,7 +312,7 @@ >>> TAROPTIONS = $(shell LC_ALL=C tar --version|grep -q GNU && echo >>> "--owner=root --group=root") >>> >>> %.o: %.c .features >>> - $(CC) -MMD $(CFLAGS) $(CPPFLAGS) $(FEATURE_CFLAGS) $(SVNDEF) -o >>> $@ -c $< >>> + $(CC) -MMD $(CFLAGS) $(CPPFLAGS) $(FEATURE_CFLAGS) $(SCMDEF) -o >>> $@ -c $< >>> >>> # Make sure to add all names of generated binaries here. >>> # This includes all frontends and libflashrom. >>> @@ -420,9 +422,12 @@ >>> >>> export: >>> @rm -rf $(EXPORTDIR)/flashrom-$(RELEASENAME) >>> - @svn export -r BASE . $(EXPORTDIR)/flashrom-$(RELEASENAME) >>> - @sed "s/^SVNVERSION.*/SVNVERSION := $(SVNVERSION)/" Makefile >>> >$(EXPORTDIR)/flashrom-$(RELEASENAME)/Makefile >>> - @LC_ALL=C svn log >$(EXPORTDIR)/flashrom-$(RELEASENAME)/ChangeLog >>> + @svn export -r BASE . $(EXPORTDIR)/flashrom-$(RELEASENAME) >>> 2>/dev/null || git checkout-index -a -f >>> --prefix=$(EXPORTDIR)/flashrom-$(RELEASENAME)/ >>> + @# If SCMVERSION or SCMEXTVERSION contain a slash or SCMURL >>> contains a hash, this will explode >>> + @sed "s/^SCMVERSION.*/SCMVERSION := >>> $(SCMVERSION)/;s/^SCMEXTVERSION.*/SCMEXTVERSION := >>> $(SCMEXTVERSION)/;s#^SCMURL.*#SCMURL := $(SCMURL)#" Makefile >>> >$(EXPORTDIR)/flashrom-$(RELEASENAME)/Makefile >>> + @# ChangeLog should be in English, but we want UTF-8 for names. >>> + @( LC_ALL=en_US.UTF-8 svn log 2>/dev/null || LC_ALL=en_US.UTF-8 >>> git log 2>/dev/null || echo "Unable to extract log from SCM" ) >>> >$(EXPORTDIR)/flashrom-$(RELEASENAME)/ChangeLog >>> + @rm -f $(EXPORTDIR)/flashrom-$(RELEASENAME)/.gitignore >>> $(EXPORTDIR)/flashrom-$(RELEASENAME)/.gitattributes >>> $(EXPORTDIR)/flashrom-$(RELEASENAME)/.gitmodules >>> @echo Exported $(EXPORTDIR)/flashrom-$(RELEASENAME)/ >>> >>> tarball: export >>> Index: flashrom-gitfriendly/flashrom.c >>> =================================================================== >>> --- flashrom-gitfriendly/flashrom.c (Revision 1145) >>> +++ flashrom-gitfriendly/flashrom.c (Arbeitskopie) >>> @@ -1355,6 +1355,7 @@ >>> void print_version(void) >>> { >>> msg_ginfo("flashrom v%s", flashrom_version); >>> + msg_ginfo(" from " FLASHROM_SCMURL ","); >>> print_sysinfo(); >>> } >>> >>> @@ -1362,7 +1363,6 @@ >>> { >>> msg_ginfo("flashrom is free software, get the source code at " >>> "http://www.flashrom.org\n"); >>> - msg_ginfo("\n"); >>> } >>> >>> int selfcheck(void) >>> >>> >>> -- >>> http://www.hailfinger.org/ >>> >>> >>> _______________________________________________ >>> flashrom mailing list >>> flashrom@flashrom.org >>> http://www.flashrom.org/mailman/listinfo/flashrom >> >> >> >> -- >> David Hendricks (dhendrix) >> Systems Software Engineer, Google Inc. > > > > -- > David Hendricks (dhendrix) > Systems Software Engineer, Google Inc. >
guys, here is the change for your review: http://codereview.chromium.org/3265003/show It does not deal with the recent addition which retrieves SVN revision number, that part can be easily added, it mostly makes the git version display much more useful. Ideally we get something like this into the tree. cheers, /vb On Fri, Aug 27, 2010 at 2:08 PM, Vadim Bendebury (вб) <vbendeb@google.com> wrote: > On Fri, Aug 27, 2010 at 1:52 PM, David Hendricks <dhendrix@google.com> wrote: >> [+vadim] >> >> On Fri, Aug 27, 2010 at 1:49 PM, David Hendricks <dhendrix@google.com> >> wrote: >>> >>> Looks pretty good so far! >>> For the benefit of everyone who missed the excitement on IRC, there is one >>> major caveat to this patch: Those using a git repository probably have stale >>> SVN metadata, which means the upstream Flashrom SVN information in the >>> version string will be incorrect. >>> For example, if I clone the Chromium OS flashrom branch (git clone >>> http://src.chromium.org/git/flashrom.git) and run "git svn info", I'll get a >>> message saying "Unable to determine upstream SVN information from working >>> tree history." After adding the upstream SVN repo as a remote repository and >>> doing "git svn fetch", I'll get something like this: >>> dhendrix@thegates:flashrom$ git svn info >>> Path: . >>> URL: svn://coreboot.org/flashrom/trunk >>> Repository Root: svn://coreboot.org/flashrom >>> Repository UUID: 2b7e53f0-3cfb-0310-b3e9-8179ed1497e1 >>> Revision: 1011 >>> Node Kind: directory >>> Schedule: normal >>> Last Changed Author: uwe >>> Last Changed Rev: 1011 >>> Last Changed Date: 2010-05-24 10:39:14 -0700 (Mon, 24 May 2010) >>> In this example, r1011 is incorrect. >>> Carl-Daniel root caused the issue and has a script that seems to >>> workaround it. Here is his blogpost which summarizes the problem and >>> workaround very >>> well: http://blogs.coreboot.org/blog/2010/08/27/git-svn-info-unable-to-determine-upstream-svn-information-from-working-tree-history/ >>> Using Carl-Daniel's script in the same repo, I get: >>> dhendrix@thegates:flashrom$ sh /tmp/cdh.sh >>> URL: svn://coreboot.org/flashrom/trunk >>> Repository UUID: 2b7e53f0-3cfb-0310-b3e9-8179ed1497e1 >>> Revision: 1145 >>> Last Changed Author: hailfinger >>> which is (currently) correct. >>> Now to figure out how best to integrate this... Can we put the script in >>> util/ and have the Makefile invoke it when it detects we're using a git >>> repo? > > guys, I am joining the party late so to speak, I have a couple of questions: > > - why do you need to integrate shell scripts into the Makefile. It is > perfectly valid to put the code in a file in ./util/ > > - when local git tree (or svn tree to that matter) has modified files > - this should be reflected in the version string, to indicate that the > code was built off modified source tree. > > - the git hash on its own is not good enough: you can't tell looking > at two hashes which one is more recent, this is why in case svn > information is not available the date should be included. In case the > source is built off pristine sources, the date should be the git log > date, in case the tree was modified, the date should be the build > date. > > - *anything* isbetter than producing multiple different images with > the same version string > > I have a patch which achieves most of that, I'll share it with you in > a bit later, > > cheers, > /vb > > > - it is much better to have a stale svn revision >>> On Sat, Aug 21, 2010 at 11:21 AM, Carl-Daniel Hailfinger >>> <c-d.hailfinger.devel.2006@gmx.net> wrote: >>>> >>>> flashrom relies on svn to export tarballs and only prints the last svn >>>> revision in the version message even if flashrom is in a git tree. >>>> >>>> Print extended version information (version, svn revision, git hash, >>>> repository URL). >>>> Allow exporting from git trees. >>>> >>>> This should make git users first-class citizens like svn users. >>>> >>>> Side note: If anyone has an idea how to make the export target more >>>> beautiful (80 column limit etc.), please tell me. >>>> >>>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> >>>> >>>> Index: flashrom-gitfriendly/Makefile >>>> =================================================================== >>>> --- flashrom-gitfriendly/Makefile (Revision 1145) >>>> +++ flashrom-gitfriendly/Makefile (Arbeitskopie) >>>> @@ -97,13 +97,15 @@ >>>> # of the checked out flashrom files. >>>> # Note to packagers: Any tree exported with "make export" or "make >>>> tarball" >>>> # will not require subversion. The downloadable snapshots are already >>>> exported. >>>> -SVNVERSION := $(shell LC_ALL=C svnversion -cn . 2>/dev/null | sed -e >>>> "s/.*://" -e "s/\([0-9]*\).*/\1/" | grep "[0-9]" || LC_ALL=C svn info . >>>> 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || LC_ALL=C git >>>> svn info . 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || >>>> echo unknown) >>>> +SCMVERSION := $(shell LC_ALL=C svnversion -cn . 2>/dev/null | sed -e >>>> "s/.*://" -e "s/\([0-9]*\).*/\1/" | grep "[0-9]" || LC_ALL=C svn info . >>>> 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || LC_ALL=C git >>>> svn info . 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || >>>> echo unknown) >>>> +SCMEXTVERSION := $(shell LC_ALL=C git rev-parse HEAD 2>/dev/null | sed >>>> 's/^./-\0/') >>>> +SCMURL := $(shell LC_ALL=C svn info 2>/dev/null | grep URL: | sed >>>> 's/.*URL:[[:blank:]]*//' | grep ^. || LC_ALL=C git remote show origin >>>> 2>/dev/null |grep URL: | sed 's/.*URL:[[:blank:]]*//' | grep ^. || echo >>>> unknown ) >>>> >>>> RELEASE := 0.9.2 >>>> -VERSION := $(RELEASE)-r$(SVNVERSION) >>>> +VERSION := $(RELEASE)-r$(SCMVERSION)$(SCMEXTVERSION) >>>> RELEASENAME ?= $(VERSION) >>>> >>>> -SVNDEF := -D'FLASHROM_VERSION="$(VERSION)"' >>>> +SCMDEF := -D'FLASHROM_VERSION="$(VERSION)"' >>>> -D'FLASHROM_SCMURL="$(SCMURL)"' >>>> >>>> # Always enable internal/onboard support for now. >>>> CONFIG_INTERNAL ?= yes >>>> @@ -310,7 +312,7 @@ >>>> TAROPTIONS = $(shell LC_ALL=C tar --version|grep -q GNU && echo >>>> "--owner=root --group=root") >>>> >>>> %.o: %.c .features >>>> - $(CC) -MMD $(CFLAGS) $(CPPFLAGS) $(FEATURE_CFLAGS) $(SVNDEF) -o >>>> $@ -c $< >>>> + $(CC) -MMD $(CFLAGS) $(CPPFLAGS) $(FEATURE_CFLAGS) $(SCMDEF) -o >>>> $@ -c $< >>>> >>>> # Make sure to add all names of generated binaries here. >>>> # This includes all frontends and libflashrom. >>>> @@ -420,9 +422,12 @@ >>>> >>>> export: >>>> @rm -rf $(EXPORTDIR)/flashrom-$(RELEASENAME) >>>> - @svn export -r BASE . $(EXPORTDIR)/flashrom-$(RELEASENAME) >>>> - @sed "s/^SVNVERSION.*/SVNVERSION := $(SVNVERSION)/" Makefile >>>> >$(EXPORTDIR)/flashrom-$(RELEASENAME)/Makefile >>>> - @LC_ALL=C svn log >$(EXPORTDIR)/flashrom-$(RELEASENAME)/ChangeLog >>>> + @svn export -r BASE . $(EXPORTDIR)/flashrom-$(RELEASENAME) >>>> 2>/dev/null || git checkout-index -a -f >>>> --prefix=$(EXPORTDIR)/flashrom-$(RELEASENAME)/ >>>> + @# If SCMVERSION or SCMEXTVERSION contain a slash or SCMURL >>>> contains a hash, this will explode >>>> + @sed "s/^SCMVERSION.*/SCMVERSION := >>>> $(SCMVERSION)/;s/^SCMEXTVERSION.*/SCMEXTVERSION := >>>> $(SCMEXTVERSION)/;s#^SCMURL.*#SCMURL := $(SCMURL)#" Makefile >>>> >$(EXPORTDIR)/flashrom-$(RELEASENAME)/Makefile >>>> + @# ChangeLog should be in English, but we want UTF-8 for names. >>>> + @( LC_ALL=en_US.UTF-8 svn log 2>/dev/null || LC_ALL=en_US.UTF-8 >>>> git log 2>/dev/null || echo "Unable to extract log from SCM" ) >>>> >$(EXPORTDIR)/flashrom-$(RELEASENAME)/ChangeLog >>>> + @rm -f $(EXPORTDIR)/flashrom-$(RELEASENAME)/.gitignore >>>> $(EXPORTDIR)/flashrom-$(RELEASENAME)/.gitattributes >>>> $(EXPORTDIR)/flashrom-$(RELEASENAME)/.gitmodules >>>> @echo Exported $(EXPORTDIR)/flashrom-$(RELEASENAME)/ >>>> >>>> tarball: export >>>> Index: flashrom-gitfriendly/flashrom.c >>>> =================================================================== >>>> --- flashrom-gitfriendly/flashrom.c (Revision 1145) >>>> +++ flashrom-gitfriendly/flashrom.c (Arbeitskopie) >>>> @@ -1355,6 +1355,7 @@ >>>> void print_version(void) >>>> { >>>> msg_ginfo("flashrom v%s", flashrom_version); >>>> + msg_ginfo(" from " FLASHROM_SCMURL ","); >>>> print_sysinfo(); >>>> } >>>> >>>> @@ -1362,7 +1363,6 @@ >>>> { >>>> msg_ginfo("flashrom is free software, get the source code at " >>>> "http://www.flashrom.org\n"); >>>> - msg_ginfo("\n"); >>>> } >>>> >>>> int selfcheck(void) >>>> >>>> >>>> -- >>>> http://www.hailfinger.org/ >>>> >>>> >>>> _______________________________________________ >>>> flashrom mailing list >>>> flashrom@flashrom.org >>>> http://www.flashrom.org/mailman/listinfo/flashrom >>> >>> >>> >>> -- >>> David Hendricks (dhendrix) >>> Systems Software Engineer, Google Inc. >> >> >> >> -- >> David Hendricks (dhendrix) >> Systems Software Engineer, Google Inc. >> >
On 28.08.2010 00:00, Vadim Bendebury wrote: > guys, here is the change for your review: > > http://codereview.chromium.org/3265003/show > I have added my review there. The suggested code breaks for me. > It does not deal with the recent addition which retrieves SVN revision > number, that part can be easily added, it mostly makes the git version > display much more useful. > > Ideally we get something like this into the tree. > What exactly was wrong with the patch I posted? > On Fri, Aug 27, 2010 at 2:08 PM, Vadim Bendebury (вб) > <vbendeb@google.com> wrote: > >> On Fri, Aug 27, 2010 at 1:52 PM, David Hendricks <dhendrix@google.com> wrote: >> >>> [+vadim] >>> >>> On Fri, Aug 27, 2010 at 1:49 PM, David Hendricks <dhendrix@google.com> >>> wrote: >>> >>>> Looks pretty good so far! >>>> For the benefit of everyone who missed the excitement on IRC, there is one >>>> major caveat to this patch: Those using a git repository probably have stale >>>> SVN metadata, which means the upstream Flashrom SVN information in the >>>> >>>> >> guys, I am joining the party late so to speak, I have a couple of questions: >> >> - why do you need to integrate shell scripts into the Makefile. It is >> perfectly valid to put the code in a file in ./util/ >> >> - when local git tree (or svn tree to that matter) has modified files >> - this should be reflected in the version string, to indicate that the >> code was built off modified source tree. >> Debatable. >> - the git hash on its own is not good enough: you can't tell looking >> at two hashes which one is more recent, this is why in case svn >> information is not available the date should be included. In case the >> source is built off pristine sources, the date should be the git log >> date, in case the tree was modified, the date should be the build >> date. >> If svn information is not available at all, the report is totally worthless anyway. >> - *anything* isbetter than producing multiple different images with >> the same version string >> Of course images with the same version string will differ. Or do you really suggest to ship debug builds only, and print the version number of every used header file? >> I have a patch which achieves most of that, I'll share it with you in >> a bit later, >> I think we need a few more iterations to get this through review. Regards, Carl-Daniel
On Mon, Aug 30, 2010 at 6:48 AM, Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote: > On 28.08.2010 00:00, Vadim Bendebury wrote: >> guys, here is the change for your review: >> >> http://codereview.chromium.org/3265003/show >> > > I have added my review there. The suggested code breaks for me. > Thank you for your comments, I will reply later on today PDT. > >> It does not deal with the recent addition which retrieves SVN revision >> number, that part can be easily added, it mostly makes the git version >> display much more useful. >> >> Ideally we get something like this into the tree. >> > > What exactly was wrong with the patch I posted? > mostly the fact that it did not provide time of the commit (or build, if built in a changed tree), which made it impossible to tell which version is newer. > >> On Fri, Aug 27, 2010 at 2:08 PM, Vadim Bendebury (вб) >> <vbendeb@google.com> wrote: >> >>> On Fri, Aug 27, 2010 at 1:52 PM, David Hendricks <dhendrix@google.com> wrote: >>> >>>> [+vadim] >>>> >>>> On Fri, Aug 27, 2010 at 1:49 PM, David Hendricks <dhendrix@google.com> >>>> wrote: >>>> >>>>> Looks pretty good so far! >>>>> For the benefit of everyone who missed the excitement on IRC, there is one >>>>> major caveat to this patch: Those using a git repository probably have stale >>>>> SVN metadata, which means the upstream Flashrom SVN information in the >>>>> >>>>> >>> guys, I am joining the party late so to speak, I have a couple of questions: >>> >>> - why do you need to integrate shell scripts into the Makefile. It is >>> perfectly valid to put the code in a file in ./util/ >>> >>> - when local git tree (or svn tree to that matter) has modified files >>> - this should be reflected in the version string, to indicate that the >>> code was built off modified source tree. >>> > > Debatable. > well, we might be coming from different angles, I think it is absolutely clear in my situation: is a development environment with multiple copies quickly changing utility flying around, being able to tell that the two versions are not exactly the same would save us a lot of time and frustration. It is less of a value in the model where the code is on the server and people just build the utility for themselves without changing it, but certainly does not hurt there either, does it? > >>> - the git hash on its own is not good enough: you can't tell looking >>> at two hashes which one is more recent, this is why in case svn >>> information is not available the date should be included. In case the >>> source is built off pristine sources, the date should be the git log >>> date, in case the tree was modified, the date should be the build >>> date. >>> > > If svn information is not available at all, the report is totally > worthless anyway. > It is of a less of a value, but certainly not worthless: it at least helps to tell that you are running something different or exactly the same as somebody else. It is especially helpful when people complaint about problems with flashrom on their systems. > >>> - *anything* isbetter than producing multiple different images with >>> the same version string >>> > > Of course images with the same version string will differ. Or do you > really suggest to ship debug builds only, and print the version number > of every used header file? > This is one deep philosophical difference :). Where I come from one can always be sure that the same version string means the same code, anything else s not acceptable. In this case it is even easily achievable, we should have this benefit. If the git tree is clean, the hash is fixed, there is no version string variations. If any file changes, we will have an indication of this fact (the + appended to the version) and a different build time each time we build. This could result in different version strings for the same code, but guarantees that different code will have different version string. Adding username of the build process would make it more robust, or we can calculate the hash of the current tree contents. This sure can and should be discussed further. >>> I have a patch which achieves most of that, I'll share it with you in >>> a bit later, >>> > > I think we need a few more iterations to get this through review. > I committed it to the chromium.org tree after an LGTM just to be able to move ahead other parts of the project, but by no means is it done, I'll fix the problem you highlighted in the review and will will reconcile with your suggested patch, stay tuned, cheers, /vb > Regards, > Carl-Daniel > > -- > http://www.hailfinger.org/ > >
Patch
Index: flashrom-gitfriendly/Makefile =================================================================== --- flashrom-gitfriendly/Makefile (Revision 1145) +++ flashrom-gitfriendly/Makefile (Arbeitskopie) @@ -97,13 +97,15 @@ # of the checked out flashrom files. # Note to packagers: Any tree exported with "make export" or "make tarball" # will not require subversion. The downloadable snapshots are already exported. -SVNVERSION := $(shell LC_ALL=C svnversion -cn . 2>/dev/null | sed -e "s/.*://" -e "s/\([0-9]*\).*/\1/" | grep "[0-9]" || LC_ALL=C svn info . 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || LC_ALL=C git svn info . 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || echo unknown) +SCMVERSION := $(shell LC_ALL=C svnversion -cn . 2>/dev/null | sed -e "s/.*://" -e "s/\([0-9]*\).*/\1/" | grep "[0-9]" || LC_ALL=C svn info . 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || LC_ALL=C git svn info . 2>/dev/null | awk '/^Revision:/ {print $$2 }' | grep "[0-9]" || echo unknown) +SCMEXTVERSION := $(shell LC_ALL=C git rev-parse HEAD 2>/dev/null | sed 's/^./-\0/') +SCMURL := $(shell LC_ALL=C svn info 2>/dev/null | grep URL: | sed 's/.*URL:[[:blank:]]*//' | grep ^. || LC_ALL=C git remote show origin 2>/dev/null |grep URL: | sed 's/.*URL:[[:blank:]]*//' | grep ^. || echo unknown ) RELEASE := 0.9.2 -VERSION := $(RELEASE)-r$(SVNVERSION) +VERSION := $(RELEASE)-r$(SCMVERSION)$(SCMEXTVERSION) RELEASENAME ?= $(VERSION) -SVNDEF := -D'FLASHROM_VERSION="$(VERSION)"' +SCMDEF := -D'FLASHROM_VERSION="$(VERSION)"' -D'FLASHROM_SCMURL="$(SCMURL)"' # Always enable internal/onboard support for now. CONFIG_INTERNAL ?= yes @@ -310,7 +312,7 @@ TAROPTIONS = $(shell LC_ALL=C tar --version|grep -q GNU && echo "--owner=root --group=root") %.o: %.c .features - $(CC) -MMD $(CFLAGS) $(CPPFLAGS) $(FEATURE_CFLAGS) $(SVNDEF) -o $@ -c $< + $(CC) -MMD $(CFLAGS) $(CPPFLAGS) $(FEATURE_CFLAGS) $(SCMDEF) -o $@ -c $< # Make sure to add all names of generated binaries here. # This includes all frontends and libflashrom. @@ -420,9 +422,12 @@ export: @rm -rf $(EXPORTDIR)/flashrom-$(RELEASENAME) - @svn export -r BASE . $(EXPORTDIR)/flashrom-$(RELEASENAME) - @sed "s/^SVNVERSION.*/SVNVERSION := $(SVNVERSION)/" Makefile >$(EXPORTDIR)/flashrom-$(RELEASENAME)/Makefile - @LC_ALL=C svn log >$(EXPORTDIR)/flashrom-$(RELEASENAME)/ChangeLog + @svn export -r BASE . $(EXPORTDIR)/flashrom-$(RELEASENAME) 2>/dev/null || git checkout-index -a -f --prefix=$(EXPORTDIR)/flashrom-$(RELEASENAME)/ + @# If SCMVERSION or SCMEXTVERSION contain a slash or SCMURL contains a hash, this will explode + @sed "s/^SCMVERSION.*/SCMVERSION := $(SCMVERSION)/;s/^SCMEXTVERSION.*/SCMEXTVERSION := $(SCMEXTVERSION)/;s#^SCMURL.*#SCMURL := $(SCMURL)#" Makefile >$(EXPORTDIR)/flashrom-$(RELEASENAME)/Makefile + @# ChangeLog should be in English, but we want UTF-8 for names. + @( LC_ALL=en_US.UTF-8 svn log 2>/dev/null || LC_ALL=en_US.UTF-8 git log 2>/dev/null || echo "Unable to extract log from SCM" ) >$(EXPORTDIR)/flashrom-$(RELEASENAME)/ChangeLog + @rm -f $(EXPORTDIR)/flashrom-$(RELEASENAME)/.gitignore $(EXPORTDIR)/flashrom-$(RELEASENAME)/.gitattributes $(EXPORTDIR)/flashrom-$(RELEASENAME)/.gitmodules @echo Exported $(EXPORTDIR)/flashrom-$(RELEASENAME)/ tarball: export Index: flashrom-gitfriendly/flashrom.c =================================================================== --- flashrom-gitfriendly/flashrom.c (Revision 1145) +++ flashrom-gitfriendly/flashrom.c (Arbeitskopie) @@ -1355,6 +1355,7 @@ void print_version(void) { msg_ginfo("flashrom v%s", flashrom_version); + msg_ginfo(" from " FLASHROM_SCMURL ","); print_sysinfo(); } @@ -1362,7 +1363,6 @@ { msg_ginfo("flashrom is free software, get the source code at " "http://www.flashrom.org\n"); - msg_ginfo("\n"); } int selfcheck(void)
flashrom relies on svn to export tarballs and only prints the last svn revision in the version message even if flashrom is in a git tree. Print extended version information (version, svn revision, git hash, repository URL). Allow exporting from git trees. This should make git users first-class citizens like svn users. Side note: If anyone has an idea how to make the export target more beautiful (80 column limit etc.), please tell me. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>