Re: bk commit into 5.0 tree (tnurnberg:1.2476) BUG#15327

[Sergei Golubchik]

Hi!

On Aug 21, Tatjana A Nuernberg wrote:
> ChangeSet@stripped, 2007-08-21 20:17:34+02:00, tnurnberg@stripped +30
> -0
>   Bug #15327: configure: --with-tcp-port option being partially ignored
>   
>   make sure that if builder configured with a non-standard (!= 3306)
>   default TCP port that value actually gets used throughout. if they
>   didn't configure a value, assume "use a sensible default", which
>   will be read from /etc/services or, failing that, from the factory
>   default. That makes the order of preference
>   - command-line option
>   - my.cnf, where applicable
>   - $MYSQL_TCP_PORT environment variable
>   - /etc/services (unless configured --with-tcp-port)
>   - default port (--with-tcp-port=... or factory default)
> 
> diff -Nrup a/client/mysql.cc b/client/mysql.cc
> --- a/client/mysql.cc	2007-04-23 16:21:59 +02:00
> +++ b/client/mysql.cc	2007-08-21 20:17:26 +02:00
> @@ -673,9 +673,12 @@ static struct my_option my_long_options[
>    {"pipe", 'W', "Use named pipes to connect to server.", 0, 0, 0, GET_NO_ARG,
>     NO_ARG, 0, 0, 0, 0, 0, 0},
>  #endif
> -  {"port", 'P', "Port number to use for connection.", (gptr*) &opt_mysql_port,
> -   (gptr*) &opt_mysql_port, 0, GET_UINT, REQUIRED_ARG, 0, 0, 0, 0, 0,
> -   0},
> +  {"port", 'P', "Port number to use for connection or 0 for default to, in order of
> preference, my.cnf, $MYSQL_TCP_PORT, "
> +#if MYSQL_PORT_DEFAULT == 0
> +   "/etc/services, "
> +#endif
> +   "built-in default.", (gptr*) &opt_mysql_port,
> +   (gptr*) &opt_mysql_port, 0, GET_UINT, REQUIRED_ARG, 0, 0, 0, 0, 0,  0},

okay, although you could've
1. wrap the line to ensure it fits in 80 columns
2. mention what the default is:

    "built-it default (" STRINGIFY_ARG(MYSQL_PORT_DEFAULT) ")."

>    {"prompt", OPT_PROMPT, "Set the mysql prompt to this value.",
>     (gptr*) &current_prompt, (gptr*) &current_prompt, 0, GET_STR_ALLOC,
>     REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
> diff -Nrup a/mysql-test/mysql-test-run-shell.sh b/mysql-test/mysql-test-run-shell.sh
> --- a/mysql-test/mysql-test-run-shell.sh	2006-11-28 21:11:42 +01:00
> +++ b/mysql-test/mysql-test-run-shell.sh	2007-08-21 20:17:27 +02:00
> @@ -17,7 +17,16 @@ USE_MANAGER=0
>  MY_TZ=GMT-3
>  TZ=$MY_TZ; export TZ # for UNIX_TIMESTAMP tests to work
>  LOCAL_SOCKET=@MYSQL_UNIX_ADDR@
> -MYSQL_TCP_PORT=@MYSQL_TCP_PORT@
> +
> +if [ -z "$MYSQL_TCP_PORT" ]; then
> +  MYSQL_TCP_PORT=@MYSQL_TCP_PORT@
> +  if [ @MYSQL_TCP_PORT_DEFAULT@ -eq 0 ]; then
> +    ESP=`getent services mysql/tcp`
> +    if [ $? -eq 0 ]; then
> +      MYSQL_TCP_PORT=`echo "$ESP"|sed -e's-^[a-z]*[ ]*\([0-9]*\)/[a-z]*$-\1-g'`
> +    fi
> +  fi
> +fi

getent is not portable, but it doesn't matter here, as
mysql-test-run-shell.sh is abandoned anyway :) 
  
>  umask 022
>  
> diff -Nrup a/scripts/mysql_config.sh b/scripts/mysql_config.sh
> --- a/scripts/mysql_config.sh	2006-12-30 21:02:05 +01:00
> +++ b/scripts/mysql_config.sh	2007-08-21 20:17:27 +02:00
> @@ -92,8 +92,18 @@ fix_path pkgincludedir include/mysql inc
>  
>  version='@VERSION@'
>  socket='@MYSQL_UNIX_ADDR@'
> -port='@MYSQL_TCP_PORT@'
>  ldflags='@LDFLAGS@'
> +
> +if [ -z "$MYSQL_TCP_PORT" ]; then
> +  MYSQL_TCP_PORT=@MYSQL_TCP_PORT@
> +  if [ @MYSQL_TCP_PORT_DEFAULT@ -eq 0 ]; then
> +    ESP=`getent services mysql/tcp`
> +    if [ $? -eq 0 ]; then
> +      MYSQL_TCP_PORT=`echo "$ESP"|sed -e's-^[a-z]*[ ]*\([0-9]*\)/[a-z]*$-\1-g'`
> +    fi
> +  fi
> +fi
> +port=$MYSQL_TCP_PORT

but it does here. getent is not portable, it shouldn't be in
mysql_config. Besides, this is just wrong. If mysql was configured with
a default port, mysql_config should show port=0, not 3306. The latter
must be shown only when mysql was explictly configured to use 3306 port.

>  # Create options 
>  # We intentionally add a space to the beginning and end of lib strings, simplifies
> replace later
> diff -Nrup a/scripts/mysql_convert_table_format.sh
> b/scripts/mysql_convert_table_format.sh
> --- a/scripts/mysql_convert_table_format.sh	2006-12-30 23:47:38 +01:00
> +++ b/scripts/mysql_convert_table_format.sh	2007-08-21 20:17:27 +02:00
> @@ -39,10 +39,30 @@ if (uc($opt_type) eq "HEAP")
>  }
>  
>  $connect_opt="";
> +
> +if ($opt_port == 0)
> +{
> +  # whether to try socket or TCP depends on value for "host", so always
> +  # passing in a default port is safe.
> +  $opt_port=$ENV{"MYSQL_TCP_PORT"};
> +  if (length($opt_port) == 0)
> +  {
> +    $opt_port=@MYSQL_TCP_PORT@;
> +    if (@MYSQL_TCP_PORT_DEFAULT@ == 0)
> +    {
> +      my $esp=getservbyname("mysql","tcp");

Any reason to do getservbyname() here ? Cannot you just pass port=0 to
libmysqlclient ? It'll do getservbyname() then, no need to duplicate the
logic.

> +      if (length($esp))
> +      {
> +        $opt_port=$esp;
> +      }
> +    }
> +  }
> +}
>  if ($opt_port)
>  {
>    $connect_opt.= ";port=$opt_port";
>  }

Regards / Mit vielen Grüssen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Developer
/_/  /_/\_, /___/\___\_\___/  MySQL GmbH, Dachauer Str. 37, D-80335 München
       <___/                  Geschäftsführer: Kaj Arnö - HRB
München 162140