Re: [3.4-bi] DESTDIR support completed

[Christian Cornelssen <ccorn at cs dot tu-berlin dot de>]

Hello,

On 2 Jan 2003, Alexandre Oliva wrote:

> On Jan  1, 2003, Christian Cornelssen <ccorn@cs.tu-berlin.de> wrote:
>
> > Here are the diffs against current mainline HEAD.
>
> Thanks for updating the patch, and sorry about not having reviewed the
> earlier versions thereof in a timely manner.
>
> >  # Create the installation directories.
> > +# $(libdir)/gcc-lib/include isn't currently searched by cpp.
> >  installdirs:
> [...]
> > +	-for d in $(libsubdir) $(bindir) $(includedir) $(infodir) $(slibdir) $(man1dir) $(man7dir); do \
> > +	  fdir=; for dir in `echo "$(DESTDIR)$$d" | tr '/' ' '`; do \
> > +	    fdir=$${fdir}/$${dir}; \
> > +	    if [ -d "$${fdir}" ] ; then true ; \
> > +	    else mkdir "$${fdir}" || break; chmod a+rx "$${fdir}"; fi ; \
> > +	  done; \
> >  	done
>
> It looks like you're dropping the creation of prefix, exec_prefix,
> libdir and libdir/gcc-lib here.  Since you're actually changing the
> code, I suppose we might as well go ahead and drop all this gunk and
> use mkinstalldirs instead.

The loop essentially functions like "mkdir -p", therefore only the
directories that are ultimate installation destinations need to be
specified. But you are right in that the loops could simply be
replaced with an "mkinstalldirs" command. I just did not want to get
boos for introducing an external tool there, and the inner loop had
already been present, so I just extended its use.

> > Index: gcc/mkheaders.in
>
> I'm not sure you want DESTDIR support here.  mkheaders is used when
> the package is already installed in the final location, not during
> make install, when DESTDIR actually matters.  I won't oppose to its
> getting in, but I can't approve this change since it's not a change in
> the build machinery code.

I have verified that it works, but I am indifferent about whether it
should get in, too.

Pro: "mkinstalldirs" already takes an optional "prefix" argument which
is not as perfectly suited for relocation purposes as DESTDIR.  Thus,
adding DESTDIR support here is logical, since otherwise "mkheaders"
would be the only point where changing prefix would still be necessary
for mere installation root changes.

One possible application: You can actually run "mkheaders" with
precisely the same command line as in the final system, but before
GCC is finally installed.  This is good for testing purposes.

On the other hand, having "mkheaders" be sensitive to a DESTDIR
environment variable may be undesirable when all files and the
script have been installed finally.  And normally, no executable is
expected to find its stuff outside the configured locations, hence
"mkheaders" does not have to be nicer.

> All other changes are ok, but we're going to need ChangeLog entries to
> put them in.

Fed into the respective */ChangeLog files?  Grmpf.  But ok.

> In general, for such a large patch, we'd need copyright assignments,
> but since the changes are mostly mechanical, I don't think they can be
> covered by copyright, so we're probably safe.  Incidentally, since
> they're not covered by copyright, we don't have to worry about
> updating the copyright notices of all modified files at this time.

I agree.  However, I have assigned copyright, so we're on the very
safe side.  (Cf. the threads related to DESTDIR patches for previous
GCC releases.)

> I suggest that we revert the change in installdirs, except for the
> addition of DESTDIR, and those in mkheaders, and address those in
> separate, smaller patches.  How does that sound to you?

"mkheaders" seems to be a separate issue, but `gcc/Makefile.in's
installdirs rule should be changed along with all other DESTDIR
patches because a new routine is needed to create the (mostly
nonexistent) DESTDIRed installdirs robustly.  Without some sort of
mkinstalldirs functionality, trying the DESTDIR support with the
most common usage, i.e. setting DESTDIR to some yet nonexistent
directory, will simply fail.  I suggest the following rule:

# Create the installation directories.
# $(libdir)/gcc-lib/include isn't currently searched by cpp.
installdirs:
	$(SHELL) ${srcdir}/mkinstalldirs $(DESTDIR)$(libsubdir)
	$(SHELL) ${srcdir}/mkinstalldirs $(DESTDIR)$(bindir)
	$(SHELL) ${srcdir}/mkinstalldirs $(DESTDIR)$(includedir)
	$(SHELL) ${srcdir}/mkinstalldirs $(DESTDIR)$(infodir)
	$(SHELL) ${srcdir}/mkinstalldirs $(DESTDIR)$(slibdir)
	$(SHELL) ${srcdir}/mkinstalldirs $(DESTDIR)$(man1dir)
	$(SHELL) ${srcdir}/mkinstalldirs $(DESTDIR)$(man7dir)

Alternatively, one command with the complete directory list would work
at least as well, but the above sequence is easier to edit.

I'll prepare the remaining ChangeLog patches and separate out the
`gcc/mkheaders.in' diff.  Many thanks for reviewing and commenting on
this.

Besides, I have successfully verified the `destdir2*.diff's on
i586-pc-linux-gnu (without Ada).  However, I'd like to see the patches
tested for platforms for which I have changed `gcc/config/*' files
(i.e. systems based on alpha, arm, ia64, mips, pa, rs6000).

Regards,

Christian Cornelssen