Re: bk commit into 5.1 tree (jonathan:1.2560) BUG#30759

[Jonathan Perkin]

* On 2007-10-31 at 22:54 GMT, Timothy Smith wrote:

> > ChangeSet@stripped, 2007-09-20 10:06:36+02:00, jonathan@stripped
> +1 -0
> >   Remove the --source-install option and instead make use of --srcdir
> >   to install system tables directly from the source tree (useful for
> >   testing purposes).  This helps clean the script up a lot and clarify
> >   the three possible ways the script can be called (using compiled-in
> >   paths, passing --basedir pointing to alternative install location,
> >   or --srcdir).  Include further tidying as well.
> >   
> >   This fixes bug#30759.

> > +  echo
> > +  echo "FATAL ERROR: Could not find $*"
> > +  echo
> > +  echo "If you compiled from source, you need to run 'make install' to"
> > +  echo "copy the software into the correct location ready for operation."
> 
> Shouldn't this mention the option of --srcdir=, so that 'make install'
> isn't needed?

As --srcdir is only really for developer use or people who know what they
are doing, I didn't want to confuse the issue for the majority of users.

> > +#
> > +# We can now find my_print_defaults.  This script supports:
> > +#
> > +#   --srcdir=path pointing to compiled source tree
> > +#   --basedir=path pointing to installed binary location
> > +#
> > +# or default to compiled-in locations.
> 
> cannot_find_file says that you can be in the top level of the extracted
> archive.  That should be supported.  But this comment doesn't include
> that option, nor does the following code.
> 
> If I do:
> 
> cd /tmp
> tar zxf mysql-PLATFORM.tar.gz
> cd mysql-PLATFORM
> ./scripts/mysql_install_db
> 
> This should work, right?  @bindir@ is going to be /usr/local/bin, not
> /tmp/mysql-PLATFORM/bin.  None of these options will work in this case,
> I think.

See scripts/make_binary_distribution, for binary releases like the example
you mention, @bindir@ is rewritten to "./bin" so the example above will
work just fine as it is the expected usage.  It's only if the user tries
to run the script from a different directory that we now correctly support
passing --basedir to accomodate that.

> > [...]
> 
> This section has the same problem.  It doesn't check if the script is run
> from the top level of an unpacked binary distro.

As above.

> > -# Set up Windows-specific paths
> > -if test "$windows" -eq 1
> > -then
> > -  mysqld="./sql/mysqld"
> > -  if test -n "$srcdir" -a -f "$srcdir/sql/share/english/errmsg.sys"
> > -  then
> > -    mysqld_opt="--language=$srcdir/sql/share/english"
> > -  else
> > -    mysqld_opt="--language=./sql/share/english"
> > -  fi
> > -fi
> 
> Is this OK?  I guess the caller would need to specify --srcdir=. in
> order to make things work like before.  I think that's OK, probably.

We only use the --windows option a couple of times in the distribution,
and each time we already pass --srcdir=. so there's no change.  Users are
not expected to use --windows and we hope to remove the option completely
in the future.  In fact, with the patch applied, it is essentially a nop
only decreasing the verbosity compared to running without the option.

> The rest looks good.  I do prefer a to write large texts using cat
> <<HERE instead of lots of echos, but probably it's not significant.

Yep, or put them in functions to make the code path easier to see at a
glance rather than having to scroll lots.

Thanks for the comments, I've tested this as thoroughly as I think is
possible, trying every combination out there, but want to ensure I haven't
missed anything.

Regards,

-- 
Jonathan Perkin, Senior Production Engineer
MySQL AB, www.mysql.com