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

Dan Liew dan at su-root.co.uk
Mon Jul 13 20:52:39 PDT 2015


Hi Hans,

> 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.

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++
```

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).

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.

Thanks,
Dan.



More information about the llvm-commits mailing list