[PATCH] Fix test-release.sh to use DESTDIR

[Hans Wennborg]

On Mon, Jul 13, 2015 at 8:52 PM, Dan Liew <dan at su-root.co.uk> wrote:
>> The patch looks great, but I don't see any reason to make the prefix
>> configurable in this script. The default value of --prefix should be
>> good, so can't we just drop it, along with the InstallPrefix variable
>> and the command-line option?
>
> The original reason I did this was because
>
> - I wanted it to be explicit that ``/usr/local`` is the default prefix
> to use when building release binaries
> - I was concerned that some packagers wouldn't like the default of
> ``/usr/local`` so I wanted to give them an option to change it
>   if they really wanted to.
>
> It seems that both the CMake and autoconf build systems use
> ``/usr/local`` as the default and thinking about it more the released
> binaries should be packaged uniformly so I'm
> happy to drop the command line option I added.

Good.

> I've had a think and initially I thought that we needed to keep the
> InstallPrefix variable because the build phases needs to know where to
> find the previously built clang
>
> i.e.
>
> ```
>         # Phase 2: Build llvmCore with newly built clang from phase 1.
>         c_compiler=$llvmCore_phase1_destdir/$InstallPrefix/bin/clang
>         cxx_compiler=$llvmCore_phase1_destdir/$InstallPrefix/bin/clang++
> ```
>
> and
>
> ```
>         # Phase 3: Build llvmCore with newly built clang from phase 2.
>         c_compiler=$llvmCore_phase2_destdir/$InstallPrefix/bin/clang
>         cxx_compiler=$llvmCore_phase2_destdir/$InstallPrefix/bin/clang++

If InstallPrefix is always /usr/local, we can just put that in there.

> it seems that clang works in these locations even though that build of
> clang does not expect to be installed there (it expects to be
> installed at /usr/local/bin/clang).

Yes, I don't think Clang cares much where it's run from. (E.g. in
Chromium we just unzip it into a dir in the developer's source tree
and run it from there.)

> This actually means that my current patch is slightly wrong. For phase
> 1 and phase 2 the build should actually set --prefix to the temporary
> install directory as before because we actually run clang from that
> directory (clang's behavior might depend on whether it exists in the
> install prefix -- I don't really know) and only the final build (phase
> 3) should use the default install prefix and use DESTDIR.
>
> I'll have a go at redoing the patch.

I don't think this is necessary. I don't think Clang cares about where
it's run from, and setting --prefix and DESTDIR differently in
different phases makes everything more complicated. I think leaving
--prefix as the default and using DESTDIR in each phase is the right
way.

Cheers,
Hans