threads / patch / 330

patchCogito chicken-and-egg problem

Subject: [PATCH] Cogito chicken-and-egg problem

## tl;dr

4 messages between Apr 26, 2005 and Apr 26, 2005. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Pavel Roskin· Apr 26, 2005, 16:06 UTC · lore
Hello!

My patch for Makefile was misapplied, so installed commit-id is still needed for "make" to succeed.

Shell commands are processed by make before being passed to the shell, and $(...) is expanded by make before new PATH is set, as it is done in the current Makefile.

Also, the dependency on commit-id was dropped from my patch for some reason. I believe it's still needed. Also, we need a dependency on cat-file, which is used by commit-id internally.

Signed-off-by: Pavel Roskin <proski@gnu.org>
Show changes to diff +2 −2
Index: Makefile
===================================================================
--- f262000f302b749e485f5eb971e6aabefbb85680/Makefile  (mode:100644 sha1:4f01bbbbb3fd0e53e9ce968f167b6dae68fcfa92)
+++ uncommitted/Makefile  (mode:100644)
@@ -87,11 +87,11 @@
 http-pull: LIBS += -lcurl
 
 
-cg-version: $(VERSION)
+cg-version: $(VERSION) commit-id cat-file
 	@echo Generating cg-version...
 	@rm -f $@
 	@echo "#!/bin/sh" > $@
-	@PATH=.:$(PATH) echo "echo \"$(shell cat $(VERSION)) ($(shell commit-id))\"" >> $@
+	@echo "echo \"$(shell cat $(VERSION)) ($(shell PATH=.:$(PATH) ./commit-id))\"" >> $@
 	@chmod +x $@
 
 install: $(PROG) $(SCRIPTS) $(SCRIPT) $(GEN_SCRIPT)
-- 
Regards,
Pavel Roskin
Daniel Barkalow· Apr 26, 2005, 16:38 UTC · re: Pavel Roskin · lore

Re: [PATCH] Cogito chicken-and-egg problem

On Tue, 26 Apr 2005, Pavel Roskin wrote:
Show 12 quoted lines
> Hello!
> 
> My patch for Makefile was misapplied, so installed commit-id is still
> needed for "make" to succeed.
> 
> Shell commands are processed by make before being passed to the shell,
> and $(...) is expanded by make before new PATH is set, as it is done in
> the current Makefile.
> 
> Also, the dependency on commit-id was dropped from my patch for some
> reason.  I believe it's still needed.  Also, we need a dependency on
> cat-file, which is used by commit-id internally.

commit-id doesn't really use cat-file; the way it calls gitXnormid.sh definitely suppresses that section. In fact, commit-id with no arguments is simply an easy-to-remember way of doing "cat .git/HEAD" using a much more complicated script. Earlier, I sent a patch to just do this, since it avoids the whole issue.

(Also, .git/HEAD is a dependancy, since you want to redo the version script if you commit; but if you don't have it, that's okay and just means you're building from a tarball, which is described completely by VERSION)

	-Daniel
*This .sig left intentionally blank*
Pavel Roskin· Apr 26, 2005, 17:02 UTC · re: Daniel Barkalow · lore

Re: [PATCH] Cogito chicken-and-egg problem

Hello, Daniel!
On Tue, 2005-04-26 at 12:38 -0400, Daniel Barkalow wrote:
Show 5 quoted lines
> > Also, the dependency on commit-id was dropped from my patch for some
> > reason.  I believe it's still needed.  Also, we need a dependency on
> > cat-file, which is used by commit-id internally.
> 
> commit-id doesn't really use cat-file; the way it calls gitXnormid.sh
There is gitXnormid.sh in the current cogito.  Please update your tree.
> definitely suppresses that section.

That's not what I see if I remove cat-file but leave commit-id in my $HOME/bin:

proski@dv:~/src/cogito$ make cg-version /home/proski/bin/commit-id: line 35: cat-file: command not found Invalid id: f9f0459b5b39cf83143c91ae39b4eaf187cf678a Generating cg-version... proski@dv:~/src/cogito$

> In fact, commit-id with no arguments
> is simply an easy-to-remember way of doing "cat .git/HEAD" using a much
> more complicated script. Earlier, I sent a patch to just do this, since it
> avoids the whole issue.
That would be fine with me.
> (Also, .git/HEAD is a dependancy, since you want to redo the version
> script if you commit; but if you don't have it, that's okay and just means
> you're building from a tarball, which is described completely by VERSION)

It should be a conditional dependency, i.e. it should be a dependency if .git exists.

It seems to me that the whole idea of including SHA1 in cg-version is broken. SHA1 is not human readable. But if the maintainers insist on having SHA1 in cg-version, I want it to be done without causing build warnings for new users.

Here's an alternative patch.
Signed-off-by: Pavel Roskin <proski@gnu.org>
Show changes to diff +10 −1
Index: Makefile
===================================================================
--- f262000f302b749e485f5eb971e6aabefbb85680/Makefile  (mode:100644 sha1:4f01bbbbb3fd0e53e9ce968f167b6dae68fcfa92)
+++ uncommitted/Makefile  (mode:100644)
@@ -87,12 +87,21 @@
 http-pull: LIBS += -lcurl
 
 
+ifneq (,$(wildcard .git))
+cg-version: $(VERSION) .git/HEAD
+	@echo Generating cg-version...
+	@rm -f $@
+	@echo "#!/bin/sh" > $@
+	@echo "echo \"$(shell cat $(VERSION)) ($(shell cat .git/HEAD))\"" >> $@
+	@chmod +x $@
+else
 cg-version: $(VERSION)
 	@echo Generating cg-version...
 	@rm -f $@
 	@echo "#!/bin/sh" > $@
-	@PATH=.:$(PATH) echo "echo \"$(shell cat $(VERSION)) ($(shell commit-id))\"" >> $@
+	@echo "echo \"$(shell cat $(VERSION))\"" >> $@
 	@chmod +x $@
+endif
 
 install: $(PROG) $(SCRIPTS) $(SCRIPT) $(GEN_SCRIPT)
 	install -m755 -d $(DESTDIR)$(bindir)
-- 
Regards,
Pavel Roskin
Daniel Barkalow· Apr 26, 2005, 17:48 UTC · re: Pavel Roskin · lore

Re: [PATCH] Cogito chicken-and-egg problem

On Tue, 26 Apr 2005, Pavel Roskin wrote:
Show 6 quoted lines
> > (Also, .git/HEAD is a dependancy, since you want to redo the version
> > script if you commit; but if you don't have it, that's okay and just means
> > you're building from a tarball, which is described completely by VERSION)
> 
> It should be a conditional dependency, i.e. it should be a dependency
> if .git exists.
True.
> It seems to me that the whole idea of including SHA1 in cg-version is
> broken.  SHA1 is not human readable.  But if the maintainers insist on
> having SHA1 in cg-version, I want it to be done without causing build
> warnings for new users.

The idea is that it should appear if you're building a version that has changes in its .git; in this case, VERSION is insufficient to tell you which revision you have installed. If I've got cogito-0.8 + three untagged commits from the remote repository + two local patches, I'd like to be able to tell if the installed version is cogito-0.8 + two of the remote patches. Possibly, it should also suppress the SHA1 if you've got a tagged head, but identifying this situation is tricky, currently. Eventually, it should be able to use a cg program to update VERSION from .git/HEAD and include SHA1 for cases which aren't directly tagged.

> Here's an alternative patch.
I like your version.
	-Daniel
*This .sig left intentionally blank*

← back to recent threads