Re: [PATCH] check for ctags utility in make_ctags
Nikolay Shaplov writes: > [ check-for-ctags-in-make_ctags_v5.diff ] Pushed with minor editorialization on the wording of the error messages. regards, tom lane
Re: [PATCH] check for ctags utility in make_ctags
В письме от воскресенье, 6 января 2019 г. 17:50:36 MSK пользователь Andrew
Dunstan написал:
> > The correct way to code this is to depend on the exit code,
> > not the text output:
> >
> > if command -v etags >/dev/null
> > then
> > : ok
> > else
> > echo etags not found
> > exit 1
> > fi
>
> more succinctly,
> command -v etags >/dev/null || { echo etags not found; exit 1;}
If it is good enough for you, then is is good for me for sure...
Imported it to the patch.
diff --git a/.gitignore b/.gitignore
index 794e35b..2dfbbe1 100644
diff --git a/src/tools/make_ctags b/src/tools/make_ctags
index 1609c07..1e71692 100755
--- a/src/tools/make_ctags
+++ b/src/tools/make_ctags
@@ -2,6 +2,9 @@
# src/tools/make_ctags
+command -v ctags >/dev/null || \
+ { echo "'ctags' utility is not found" 1>&2; exit 1;}
+
trap "rm -f /tmp/$$" 0 1 2 3 15
rm -f ./tags
diff --git a/src/tools/make_etags b/src/tools/make_etags
index 3ce96bc..6dc6710 100755
--- a/src/tools/make_etags
+++ b/src/tools/make_etags
@@ -2,6 +2,9 @@
# src/tools/make_etags
+command -v etags >/dev/null || \
+ { echo "'etags' utility is not found" 1>&2; exit 1;}
+
rm -f ./TAGS
find `pwd`/ -type f -name '*.[chyl]' -print |
Re: [PATCH] check for ctags utility in make_ctags
On 1/6/19 12:16 PM, Tom Lane wrote:
>
> The correct way to code this is to depend on the exit code,
> not the text output:
>
> if command -v etags >/dev/null
> then
> : ok
> else
> echo etags not found
> exit 1
> fi
more succinctly,
command -v etags >/dev/null || { echo etags not found; exit 1;}
> We could alternatively try to use "which" in the same way,
> but I'm dubious that it's more portable than "command".
> (AFAICT, "which" is *not* in POSIX.)
>
>
Indeed. I know I have some systems where it's lacking.
cheers
andrew
--
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: [PATCH] check for ctags utility in make_ctags
Nikolay Shaplov writes: > В письме от четверг, 3 января 2019 г. 12:52:36 MSK пользователь Peter > Eisentraut написал: >> I don't know how portable command -v is. Some systems have a /bin/sh >> that is pre-POSIX. Same with $(...). > Do you know how to obtain such a shell in Debian? TBH, when I first saw this patch, I had the same reaction as Peter, ie I wondered how portable this was. However, upon investigation: 1. "command -v " is specified by Single Unix Spec v2, which we've considered as our baseline portability requirement for a good long time now. 2. Even my pet dinosaur HPUX 10.20 box recognizes it. I do not believe anybody working on PG these days is using something older. 3. These scripts aren't part of any build or runtime process, they're only useful for development. We've long felt that it's okay to have higher requirements for development environments than for production. Besides, do you really think anybody's going to be doing PG v12+ development on a box with a pre-SUSv2 shell and a C99 compiler? We need not get into the question of whether $(...) is portable, because the way it's being used is not: if command -v does not find the target command, it prints nothing, so that at least some systems will do this: $ if [ ! $(command -v notetags) ] > then > echo not found > fi ksh: test: argument expected (I'm not very sure why bash fails to act that way, actually. "!" with nothing after it shouldn't be valid syntax for test(1), you'd think.) The correct way to code this is to depend on the exit code, not the text output: if command -v etags >/dev/null then : ok else echo etags not found exit 1 fi We could alternatively try to use "which" in the same way, but I'm dubious that it's more portable than "command". (AFAICT, "which" is *not* in POSIX.) regards, tom lane
Re: [PATCH] check for ctags utility in make_ctags
В письме от четверг, 3 января 2019 г. 12:52:36 MSK пользователь Peter Eisentraut написал: > >> +1, let's keep it simple. I would just use "ctags/etags not found" > >> as error message. > > > > Actually I was trying to say "Please install 'ctags' [utility] to run > > make_ctags". But if all of you read it as "Please install 'ctags' > > [package] to run make_ctags", then it is really better to drop the > > advice. > > > > So I removed it. See the patch. > > A few more comments. > > I don't know how portable command -v is. Some systems have a /bin/sh > that is pre-POSIX. Same with $(...). Do you know how to obtain such a shell in Debian? I have dash for sh, and it knows both commands -v and $(). And I have no idea how to get more simple one. Do you have one? Do you know the way how to check if shell is pre-POSIX and just disable the check in this case. Or can you offer some another check that will satisfy you as a potential user of pre-POSIX shell? The check that will somehow report that ctags _executable_ file is missing. > If etags is not installed, the current script prints > > xargs: etags: No such file or directory make_ctags prints xargs: ctags: No such file or directory sort: cannot read: tags: No such file or directory For me it is not good enough error message, it says it can't find some ctags| etags file. But says nothing that is is an utility, that is missing... So I would try to find better way to report that ctags utility is missing. PS Vitus, I added you to CC, because I know that you are quite good in bash scripting, may be you would give some good ideas I do not have.
Re: [PATCH] check for ctags utility in make_ctags
On 03/01/2019 12:15, Nikolay Shaplov wrote: >> +1, let's keep it simple. I would just use "ctags/etags not found" >> as error message. > > Actually I was trying to say "Please install 'ctags' [utility] to run > make_ctags". But if all of you read it as "Please install 'ctags' [package] > to > run make_ctags", then it is really better to drop the advice. > > So I removed it. See the patch. A few more comments. I don't know how portable command -v is. Some systems have a /bin/sh that is pre-POSIX. Same with $(...). If etags is not installed, the current script prints xargs: etags: No such file or directory I don't see the need to do more than that, especially if it makes the script twice as long. (Personally, I'd recommend removing make_etags altogether and using GNU Global for Emacs.) -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: [PATCH] check for ctags utility in make_ctags
В письме от четверг, 3 января 2019 г. 10:03:53 MSK пользователь Michael Paquier написал: > On Wed, Jan 02, 2019 at 11:35:46AM -0500, Tom Lane wrote: > > In fact, that's demonstrably not so: on my RHEL6 and Fedora boxes, > > /usr/bin/etags isn't owned by any package, because it's a symlink > > managed by the "alternatives" system. It points to /usr/bin/etags.emacs > > which is owned by the emacs-common package. So dropping the advice > > about how to fix the problem seems like a good plan. > > +1, let's keep it simple. I would just use "ctags/etags not found" > as error message. Actually I was trying to say "Please install 'ctags' [utility] to run make_ctags". But if all of you read it as "Please install 'ctags' [package] to run make_ctags", then it is really better to drop the advice. So I removed it. See the patch. diff --git a/src/tools/make_ctags b/src/tools/make_ctags index 1609c07..0834468 100755 --- a/src/tools/make_ctags +++ b/src/tools/make_ctags @@ -2,6 +2,12 @@ # src/tools/make_ctags +if [ ! $(command -v ctags) ] +then + echo "'ctags' utility is not found" 1>&2 + exit 1 +fi + trap "rm -f /tmp/$$" 0 1 2 3 15 rm -f ./tags diff --git a/src/tools/make_etags b/src/tools/make_etags index 3ce96bc..cb44aed 100755 --- a/src/tools/make_etags +++ b/src/tools/make_etags @@ -2,6 +2,12 @@ # src/tools/make_etags +if [ ! $(command -v etags) ] +then + echo "'etags' utility is not found" 1>&2 + exit 1 +fi + rm -f ./TAGS find `pwd`/ -type f -name '*.[chyl]' -print |
Re: [PATCH] check for ctags utility in make_ctags
On Wed, Jan 02, 2019 at 11:35:46AM -0500, Tom Lane wrote: > In fact, that's demonstrably not so: on my RHEL6 and Fedora boxes, > /usr/bin/etags isn't owned by any package, because it's a symlink > managed by the "alternatives" system. It points to /usr/bin/etags.emacs > which is owned by the emacs-common package. So dropping the advice > about how to fix the problem seems like a good plan. +1, let's keep it simple. I would just use "ctags/etags not found" as error message. -- Michael signature.asc Description: PGP signature
Re: [PATCH] check for ctags utility in make_ctags
Peter Eisentraut writes: > On 01/01/2019 17:44, Nikolay Shaplov wrote: >> +if [ ! $(command -v ctags) ] >> +then >> + echo "'ctags' utility is not found" 1>&2 >> + echo "Please install 'ctags' to run make_ctags" 1>&2 >> + exit 1 >> +fi > This assumes that the ctags and etags programs are part of packages of > the same name. I don't think that is always the case. In fact, that's demonstrably not so: on my RHEL6 and Fedora boxes, /usr/bin/etags isn't owned by any package, because it's a symlink managed by the "alternatives" system. It points to /usr/bin/etags.emacs which is owned by the emacs-common package. So dropping the advice about how to fix the problem seems like a good plan. regards, tom lane
Re: [PATCH] check for ctags utility in make_ctags
On 01/01/2019 17:44, Nikolay Shaplov wrote: > +if [ ! $(command -v ctags) ] > +then > + echo "'ctags' utility is not found" 1>&2 > + echo "Please install 'ctags' to run make_ctags" 1>&2 > + exit 1 > +fi This assumes that the ctags and etags programs are part of packages of the same name. I don't think that is always the case. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: [PATCH] check for ctags utility in make_ctags
В письме от вторник, 1 января 2019 г. 11:24:11 MSK пользователь Michael Paquier написал: > Not sure if that's something worse bothering about, but you could do > the same in src/tools/make_etags. Good idea. Done. (I did not do it in the first place because I do not use etags and can't properly check it, but really if some files are created, then everything should be working well. This is good enough check :-) ) diff --git a/.gitignore b/.gitignore index 794e35b..ca14134 100644 diff --git a/src/tools/make_ctags b/src/tools/make_ctags index 1609c07..052d1e4 100755 --- a/src/tools/make_ctags +++ b/src/tools/make_ctags @@ -2,6 +2,13 @@ # src/tools/make_ctags +if [ ! $(command -v ctags) ] +then + echo "'ctags' utility is not found" 1>&2 + echo "Please install 'ctags' to run make_ctags" 1>&2 + exit 1 +fi + trap "rm -f /tmp/$$" 0 1 2 3 15 rm -f ./tags diff --git a/src/tools/make_etags b/src/tools/make_etags index 3ce96bc..68738e4 100755 --- a/src/tools/make_etags +++ b/src/tools/make_etags @@ -2,6 +2,13 @@ # src/tools/make_etags +if [ ! $(command -v etags) ] +then + echo "'etags' utility is not found" 1>&2 + echo "Please install 'etags' to run make_etags" 1>&2 + exit 1 +fi + rm -f ./TAGS find `pwd`/ -type f -name '*.[chyl]' -print |
Re: [PATCH] check for ctags utility in make_ctags
On Mon, Dec 31, 2018 at 07:19:39PM +0300, Nikolay Shaplov wrote: > В письме от понедельник, 31 декабря 2018 г. 19:04:08 MSK пользователь Nikolay > Shaplov написал: > >> I'd like to propose a small patch for make_ctags script. It checks if ctags >> utility is intalled or not. If not it reports an error and advises to >> install ctags. > > Oups. I've misplaced '&' character :-) Not sure if that's something worse bothering about, but you could do the same in src/tools/make_etags. -- Michael signature.asc Description: PGP signature
Re: [PATCH] check for ctags utility in make_ctags
В письме от понедельник, 31 декабря 2018 г. 19:04:08 MSK пользователь Nikolay Shaplov написал: > I'd like to propose a small patch for make_ctags script. It checks if ctags > utility is intalled or not. If not it reports an error and advises to > install ctags. Oups. I've misplaced '&' character :-) Here is the right version diff --git a/src/tools/make_ctags b/src/tools/make_ctags index 1609c07..edbcd82 100755 --- a/src/tools/make_ctags +++ b/src/tools/make_ctags @@ -2,6 +2,13 @@ # src/tools/make_ctags +if [ ! $(command -v ctags) ] +then + echo "'ctags' utility is not found" 1>&2 + echo "Please install 'ctags' to run make_ctags" 1>&2 + exit 1 +fi + trap "rm -f /tmp/$$" 0 1 2 3 15 rm -f ./tags
