Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 364257 - app-admin/eselect-postgresql-1.0.7: missing double quotes
Summary: app-admin/eselect-postgresql-1.0.7: missing double quotes
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: [OLD] Unspecified (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: PgSQL Bugs
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-20 15:51 UTC by Jimmy.Jazz
Modified: 2011-05-01 14:40 UTC (History)
0 users

See Also:
Package list:
Runtime testing required: ---


Attachments
normal behaviour (eselect_out_good.txt,30.98 KB, text/plain)
2011-04-22 09:11 UTC, Tomáš Chvátal (RETIRED)
Details
bad behaviour (eselect_out_bad.txt,21.10 KB, text/plain)
2011-04-22 09:11 UTC, Tomáš Chvátal (RETIRED)
Details
fixed behaviour (eselect_out_fixed.txt,30.87 KB, text/plain)
2011-04-22 09:12 UTC, Tomáš Chvátal (RETIRED)
Details
avoid_pattern_expansion.patch (avoid_pattern_expansion.patch,883 bytes, patch)
2011-04-22 09:14 UTC, Tomáš Chvátal (RETIRED)
Details | Diff
4 examples (example-find.txt,921 bytes, text/plain)
2011-04-27 13:17 UTC, Jimmy.Jazz
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jimmy.Jazz 2011-04-20 15:51:55 UTC
eselect postgresql update fails when executed in a directory containing lib* files.

--- /tmp/postgresql.eselect	2011-04-20 17:45:54.980963825 +0200
+++ /usr/share/eselect/modules/postgresql.eselect	2011-04-20 17:46:21.891047426 +0200
@@ -194,7 +194,7 @@
 			echo "${B_PATH}/${x}/postgresql" >> "${E_PATH}"/active.links
 			# Linker works for files
 			linker "${B_PATH}/${x}/postgresql-${SLOT}/${x}" \
-				"-name lib*" "${B_PATH}/${x}"
+				"-name \"lib*\"" "${B_PATH}/${x}"
 		fi
 	done


Reproducible: Always
Comment 1 Jimmy.Jazz 2011-04-20 15:53:37 UTC
++ set +x
Setting 9.0 as the default installation...
	Removing old links...done.
	Generating new links...++ find /usr/lib64/postgresql-9.0/lib64 -maxdepth 1 -mindepth 1 -name lib lib32 lib64
                    ^^^^^^^^^^^^^^^^
find: paths must precede expression: lib32
Usage: find [-H] [-L] [-P] [-Olevel] [-D help|tree|search|stat|rates|opt|exec] [path...] [expression]
+ set +x
Comment 2 Aaron W. Swenson gentoo-dev 2011-04-21 00:20:12 UTC
Please post 'emerge --info'.

Also, modify /usr/share/eselect/modules/postgresql.eselect and insert

    set-x

at line 1.

Then:

    postgresql-config update &> ~/postgresql_eselect_debug

Then attach 'postgresql_eselect_debug'.
Comment 3 Jimmy.Jazz 2011-04-21 02:10:29 UTC
(In reply to comment #2)
> Please post 'emerge --info'.
> 
> Also, modify /usr/share/eselect/modules/postgresql.eselect and insert
> 
>     set-x

I cannot be more explicit than my two comments above. Please, make an effort to read before posting.

line 197 in function linker() you forgot double quotes to avoid evaluation of lib* ...

4 am, I'm still awake and more present than you are :).
Comment 4 Aaron W. Swenson gentoo-dev 2011-04-21 03:50:35 UTC
+ linker /usr/lib/postgresql-8.4/lib '-name lib*' /usr/lib
+ local source_dir=/usr/lib/postgresql-8.4/lib
+ local 'pattern=-name lib*'
+ local target_dir=/usr/lib
+ local suffix=
+ local link_source
++ find /usr/lib/postgresql-8.4/lib -maxdepth 1 -mindepth 1 -name 'lib*'

That 'set -x' at the shell only examines what happens at the shell. It isn't evaluating the script. From the above, you can see the script is properly quoting and not expanding 'lib*'.

The portion of the script that calls the linker() function is within a for() loop that should only pass one of lib, lib32 or lib64 to the variable ${x} which is used in the path (source_dir). There's no reason for it to spew 'lib lib32 lib64' after the "find /usr/lib64/postgresql-9.0/lib64 -maxdepth 1 -mindepth 1 -name 'lib*'". Afterall, 'lib*' isn't a variable, and at that depth, it is looking for files beginning with lib (i.e., libpq.a, lippq.so), not directories beginning with 'lib'.

Which is precisely why I want to see what the script is doing on your system and why it is getting confused.
Comment 5 Jimmy.Jazz 2011-04-21 12:19:02 UTC
(In reply to comment #4)

> which is used in the path (source_dir). There's no reason for it to spew 'lib
> lib32 lib64' 

Stop arguing and apply the patch please. 

after the "find /usr/lib64/postgresql-9.0/lib64 -maxdepth 1
> -mindepth 1 -name 'lib*'

As I see. you put the quotes as I tell you. Just do it double... and backslashed in your script. See first post.

". Afterall, 'lib*' isn't a variable

Are you kidding, that not the matter. Do you believe I don't know my job. what does ls *. bash interpretes *

, and at that
> depth, it is looking for files beginning with lib (i.e., libpq.a, lippq.so),
> not directories beginning with 'lib'.

That's not the matter.

anyway , -name a b c is false and fails. You don't have to execute a script in debug mode for that. Read the script.

example: cd /

find . -name lib*

find: les chemins doivent précéder l'expression : lib32
Utilisation : find [-H] [-L] [-P] [-Olevel] [-D help|tree|search|stat|rates|opt|exec] [chemin...] [expression]

As I tell you.

> 
> Which is precisely why I want to see 

Do It yourself. I become tired to try to help you. I can apply it myself in my own overlay...
You just managed to annoy me.

what the script is doing on your system
> and why it is getting confused.

Regards
Comment 6 Tomáš Chvátal (RETIRED) gentoo-dev 2011-04-22 09:11:10 UTC
If you take look at the script and see what it is really doing:

+ linker /usr/lib64/postgresql-9.0/lib64 '-name lib*' /usr/lib64
+ local source_dir=/usr/lib64/postgresql-9.0/lib64
+ local 'pattern=-name lib*'
+ local target_dir=/usr/lib64
+ local suffix=
+ local link_source
++ find /usr/lib64/postgresql-9.0/lib64 -maxdepth 1 -mindepth 1 -name 'lib*'

You see that lib* quoted in single quotes which is correct syntax for find.
Same as would lib\* would do...

For your fancy patch the resulting find would be:
++ find /usr/lib64/postgresql-9.0/lib64 -maxdepth 1 -mindepth 1 -name '"lib*"'

Which results in complete garbge because does not return anything.

But since your bug is technically legit as the * gets expanded when passed to find in enviroment where the anything starting lib exists, we need to find proper patch that actualy works. So what we want to do is avoid non-required bash pattern expansion which is done bit differently.
Comment 7 Tomáš Chvátal (RETIRED) gentoo-dev 2011-04-22 09:11:35 UTC
Created attachment 270855 [details]
normal behaviour
Comment 8 Tomáš Chvátal (RETIRED) gentoo-dev 2011-04-22 09:11:53 UTC
Created attachment 270857 [details]
bad behaviour
Comment 9 Tomáš Chvátal (RETIRED) gentoo-dev 2011-04-22 09:12:08 UTC
Created attachment 270859 [details]
fixed behaviour
Comment 10 Tomáš Chvátal (RETIRED) gentoo-dev 2011-04-22 09:14:31 UTC
Created attachment 270861 [details, diff]
avoid_pattern_expansion.patch

Patch fixing the described issue properly
Comment 11 Tomáš Chvátal (RETIRED) gentoo-dev 2011-04-22 09:16:13 UTC
So now my dear user, you can start wearing two hats:
1) I am acting like a dick in a bugzilla
2) I don't understand bash properly even if I act like master of the universe

Cheers
Comment 12 Aaron W. Swenson gentoo-dev 2011-04-22 10:57:20 UTC
  22 Apr 2011; Aaron W. Swenson <titanofold@gentoo.org>
  -eselect-postgresql-1.0.6.ebuild, -eselect-postgresql-1.0.7.ebuild,
  +eselect-postgresql-1.0.8.ebuild, metadata.xml:
  Fixes bug 364257 and bug 364237
Comment 13 Jimmy.Jazz 2011-04-22 15:48:25 UTC
(In reply to comment #11)
> So now my dear user, you can start wearing two hats:
> 1) I am acting like a dick in a bugzilla
> 2) I don't understand bash properly even if I act like master of the universe
> 
> Cheers

Thank you for the sterile insult and provocation. That's high gentoo quality service.

The discussion was about double-quotes. set -f is too bashism
And are you able to find the missing command ?

fun() {
 local source_dir=$1
 local pattern=$2

 echo find "${source_dir}" -maxdepth 1 -mindepth 1 ${pattern}
 
 eval find "${source_dir}" -maxdepth 1 -mindepth 1 ${pattern}
}

echo Your version:
fun / '-name lib*'

echo
echo "My version:"
fun / "-name \"lib*\""


Answer for you, 'eval'
----------------------
Comment 14 Jimmy.Jazz 2011-04-27 13:10:43 UTC
@Aaron

Four useful examples.

As you are not responsible for your friend behaviour and I like gentoos, I send to you four examples how to avoid set -f and eval when you pass arguments to a function. Remember, '-name val' isn't  as '-name' 'val'. So you don't need to be geek for that :). If you find some weaknesses in the examples, just let me know. See attachment.


funA() {
	local a=$1
	local b=$2
	local c="$3"
	local l

	echo find "$a" -maxdepth 1 -mindepth 1 $b ${c:+-name "$c"}
	l=$(command find "$a" -maxdepth 1 -mindepth 1 $b ${c:+-name "$c"})

	echo $l
}

funB() {
	local a=$1
	local b=$2
	local c="$3"
	local l

	echo find "$a" -maxdepth 1 -mindepth 1 $b $c
	l=$(eval command find "$a" -maxdepth 1 -mindepth 1 $b $c)

	echo $l
}

find() {
	local a=$1
	local c="$2"
	local l

	echo find "$a" -maxdepth 1 -mindepth 1 ${c:+-name} "$c"
	l=$(command find "$a" -maxdepth 1 -mindepth 1 $b ${c:+-name "$c"})

	echo $l
}

# arg b is only to illustrate an "unused" argument
# command avoid loops and collisions when the same name is involved for both a function and a command

cd /
echo 'working funA()'
funA '/' '' 'lib*'

echo 'working funB() with eval'
funB '/' '' "-name 'lib*'"

echo 'working find() <-- my favorite'
find '/' 'lib*'

echo 'working find() <-- my favorite'
find '/' ""

You get,

working funA()
find / -maxdepth 1 -mindepth 1 -name lib*
/lib64 /lib32 /lib
working funB() with eval
find / -maxdepth 1 -mindepth 1 -name 'lib*'
/lib64 /lib32 /lib
working find() <-- my favorite
find / -maxdepth 1 -mindepth 1 -name lib*
/lib64 /lib32 /lib
working find() <-- my favorite
find / -maxdepth 1 -mindepth 1 
/service /boot /media /tmp /.viminfo /root /mnt /lib64 /cgroup /lib32
Comment 15 Jimmy.Jazz 2011-04-27 13:17:47 UTC
Created attachment 271337 [details]
4 examples
Comment 16 Aaron W. Swenson gentoo-dev 2011-05-01 14:40:42 UTC
Thank you for your hard work, but none of those are quite right to suit the needs of the module.

The first example forces the '-name' option, the second example utilizes eval, and the third does the same as the first by forcing the use of the '-name' option. I use more than just the '-name' option throughout the module.

Using 'set -/+f' on the other hand is quite alright as it is a Bash built-in, and does precisely what is needed to avoid expansion of the arguments.

And there's no such thing as being "too Bash-ism" when the modules are explicitly Bash, so there are no compatibility worries there.