MySQL Bugs: #27701: Args not properly passed in compile-pentium-* scripts

Bug #27701	Args not properly passed in compile-pentium-* scripts
Submitted:	8 Apr 2007 0:43	Modified:	27 Apr 2007 15:16
Reporter:	jocelyn fournier (Silver Quality Contributor)	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Compiling	Severity:	S3 (Non-critical)
Version:	5.0,5.1	OS:	Any
Assigned to:	Daniel Fischer	CPU Architecture:	Any

Description:
Hi,

Currently the compile-pentium-debug uses the syntax 

. "$path/SETUP.sh" $@ --with-debug=full 

to call SETUP.sh.
This doesn't seem to work properly (at least on ubuntu) (the extra args are ignored since the "." imply SETUP.sh is executed in the same shell).
Hence --with-debug=full is not taken into account, and the program is compiled with -O1 flag.

Regards,
  Jocelyn

How to repeat:
Execute compile-pentium-debug script on ubuntu 6.10 or 7.04.
You will see the code will be compile with -O1 -Wuninitialized when means --with-debug=full is not taken into account.

Thank you for the report.

Please provide output of /bin/sh --version and echo $SHELL

Hi,

Ok I now understand what's wrong : ubuntu edgy & feisty are using dash instead of bash by default. (sh is a symlink to dash)
Changing the compile-pentium-* scripts to specify explicitly

#! /bin/bash

instead of

#! /bin/sh

solve the issue.

(and that's actually what's recommanded according to https://bugs.launchpad.net/ubuntu/+source/dash/+bug/61463)

Regards,
  Jocelyn

Thank you for the feedback.

Verified as described as really bash is needed.

Change /bin/sh to /bin/bash

Attachment: patch-27701 (application/octet-stream, text), 16.85 KiB.

Hi,

I've just uploaded a patch for 5.1 to change /bin/sh to /bin/bash in the BUILD directory.

Regards,
  Jocelyn

The above patch just changes all occurrances of /bin/sh to /bin/bash. That's not really a good idea as we can't rely on bash being available on all systems these scripts might run on. Additionally only five files are affected but this patch changes almost all scripts in BUILD.

There's a second problem with the currently employed method of modifying the arguments for the sourced script; arguments with whitespace in them might be broken up into several arguments.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/24187

ChangeSet@1.2437, 2007-04-10 17:50:43+02:00, df@pippilotta.erinye.com +5 -0
  BUG#27701 Arguments to some compile-pentium* scripts were not properly passed to SETUP.sh. Besides the old way not working with some shells, single arguments that contained whitespace were also broken up. This patch tries to fix both errors.

Hi,

For the other compile* scripts, the change to set $@ is also needed, to handle args with spaces ?

Thanks,
  Jocelyn

set -- "$@" (the important part is the "") is a no-op. Using "$@" is only necessary when modifying/passing parameters.

Hi Daniel,

Other scripts are using 

. "$path/SETUP.sh" $@

syntax, but actually the $@ is not needed since it uses the same shell and hence the same args.

  Jocelyn

Sorry, you're right, in 5.1 there are scripts that use $@ and nothing else. The above patch is for 5.0. In 5.1 the single $@ will be removed.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/24273

ChangeSet@1.2576, 2007-04-11 14:12:00+02:00, df@pippilotta.erinye.com +4 -0
  BUG#27701 don't pass arguments to sourced script if they're not modified as this is either a no-op (if done correctly), a different no-op with some shells (if done the bash way, but with correct quoting) or breaks arguments with whitespace for some shells (if done the bash way, without quotes).

Patches (5.0 + 5.1) are ok with me.  
Please try to get the reporter to verify as well.

queued in 5.0-build and 5.1-build

Pushed into 5.1.18-beta

Pushed into 5.0.42

No changelog entry needed.