Re: [Openocd-development] [PATCH] A fix (was Re: Git revision string missing from banner)

2011-08-16 Thread Jie Zhang
On Mon, Aug 15, 2011 at 6:21 PM, Andreas Fritiofson
andreas.fritiof...@gmail.com wrote:
   +if test -f $srcdir/guess-rev.sh ; then
 Great, but you should probably check if it's executable instead of just a
 regular file.

I considered if we should check this. But I finally decided it would
be better to just check if it's a regular file. my reason is:

We decide if we are building for release or not by the existence of
guess-rev.sh, not by the existence of an executable guess-rev.sh. So

1. guess-rev.sh exists and is executable: checking regular file is
same as checking executable file.
2. guess-rev.sh does not exist: checking regular file is same as
checking executable file.
3. guess-rev.sh exists but is not executable: there must be something
wrong. If we check regular file, user will be given an error when
guess-rev.sh is going to be executed on compiler time. If we check
executable file, building for release is assumed and user will notice
it only in run time.

Regards,
Jie
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development


Re: [Openocd-development] [PATCH] A fix (was Re: Git revision string missing from banner)

2011-08-16 Thread Andreas Fritiofson
On Tue, Aug 16, 2011 at 4:19 PM, Jie Zhang jzhang...@gmail.com wrote:

 On Mon, Aug 15, 2011 at 6:21 PM, Andreas Fritiofson
 andreas.fritiof...@gmail.com wrote:
+if test -f $srcdir/guess-rev.sh ; then
  Great, but you should probably check if it's executable instead of just a
  regular file.

 I considered if we should check this. But I finally decided it would
 be better to just check if it's a regular file. my reason is:

 We decide if we are building for release or not by the existence of
 guess-rev.sh, not by the existence of an executable guess-rev.sh. So

 1. guess-rev.sh exists and is executable: checking regular file is
 same as checking executable file.
 2. guess-rev.sh does not exist: checking regular file is same as
 checking executable file.
 3. guess-rev.sh exists but is not executable: there must be something
 wrong. If we check regular file, user will be given an error when
 guess-rev.sh is going to be executed on compiler time. If we check
 executable file, building for release is assumed and user will notice
 it only in run time.


Agreed, failure is probably the better option if the permissions are
botched. I'll leave it as is.

/Andreas
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development


Re: [Openocd-development] [PATCH] A fix (was Re: Git revision string missing from banner)

2011-08-15 Thread Spencer Oliver
On 15 August 2011 16:03, Jie Zhang jzhang...@gmail.com wrote:
 On Mon, Aug 15, 2011 at 9:35 AM, Spencer Oliver s...@spen-soft.co.uk wrote:
 On 15 August 2011 14:22, Eric Wetzel thewet...@gmail.com wrote:
 if test $cross_compiling = no; then
  # guess-rev.sh only exists in the repository, not in the released archives
  AC_CHECK_FILE($srcdir/guess-rev.sh, has_guess_rev=yes, has_guess_rev=no)

 We automatically assume that if we are cross-compiling that we are
 building a release. This section was last modified in July 2009, back
 in the SVN days, but the commit is here:
 http://repo.or.cz/w/openocd.git/commitdiff/00fad24996d6642c6a820cc951c197dddef5734a

 Actually it was introduced earlier

 http://repo.or.cz/w/openocd.git/commit/64e53c4fd8a5da944de57716b137a7dff5e67b63

 This patch should fix it. Please review and merge if it's OK. Thank you!


looks fine to me.

thanks
spen
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development


Re: [Openocd-development] [PATCH] A fix (was Re: Git revision string missing from banner)

2011-08-15 Thread Andreas Fritiofson
On Mon, Aug 15, 2011 at 5:03 PM, Jie Zhang jzhang...@gmail.com wrote:

 On Mon, Aug 15, 2011 at 9:35 AM, Spencer Oliver s...@spen-soft.co.uk
 wrote:
  On 15 August 2011 14:22, Eric Wetzel thewet...@gmail.com wrote:
  if test $cross_compiling = no; then
   # guess-rev.sh only exists in the repository, not in the released
 archives
   AC_CHECK_FILE($srcdir/guess-rev.sh, has_guess_rev=yes,
 has_guess_rev=no)
 
  We automatically assume that if we are cross-compiling that we are
  building a release. This section was last modified in July 2009, back
  in the SVN days, but the commit is here:
 
 http://repo.or.cz/w/openocd.git/commitdiff/00fad24996d6642c6a820cc951c197dddef5734a
 
 Actually it was introduced earlier


 http://repo.or.cz/w/openocd.git/commit/64e53c4fd8a5da944de57716b137a7dff5e67b63

 This patch should fix it. Please review and merge if it's OK. Thank you!

 Jie


  +if test -f $srcdir/guess-rev.sh ; then

Great, but you should probably check if it's executable instead of just a
regular file.

/Andreas
___
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development