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-u... 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%5Cn");
- msg_ginfo("\n");
}
int selfcheck(void)
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.