sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 482 forks source link

Add DESTDIR support for optional packages that use autotools #27039

Closed embray closed 5 years ago

embray commented 5 years ago

All these packages have standard autotools builds, so their transformation was mostly mechanical and trivial. Only two slight exceptions:

I've tested all of these and confirmed that they still build properly, and that the files listed in their file manifests look logical, and doctests pass for me on Linux. There should really be no difference for these packages compared to how they were previously built and installed.

Component: packages: optional

Keywords: destdir 4ti2 bliss cbc csdp deformation fricas gp2c igraph latte_int libogg lidia lrslib mpfrcx sirocco tdlib

Author: Erik Bray

Branch: 1747a4c

Reviewer: Frédéric Chapoton

Issue created by migration from https://trac.sagemath.org/ticket/27039

embray commented 5 years ago

Description changed:

--- 
+++ 
@@ -4,4 +4,4 @@

 * For mpfrcx we also add an `spkg-legacy-uninstall` script.

-I've tested all of these and confirmed that they still build properly, and that the files listed in their file manifests look logical.  Still running the doctests to be sure, but there should really be no difference for these packages compared to how they were previously built and installed.
+I've tested all of these and confirmed that they still build properly, and that the files listed in their file manifests look logical, and doctests pass for me on Linux. There should really be no difference for these packages compared to how they were previously built and installed.
fchapoton commented 5 years ago
comment:2

In tdlib, you added sdh_make where there was no make. Is this ok ?

Otherwise, positive review

embray commented 5 years ago
comment:3

Replying to @fchapoton:

In tdlib, you added sdh_make where there was no make. Is this ok ?

Otherwise, positive review

I double-checked on this just to be sure, but it is indeed a standard automake package so it is harmless to add the make call, and it looks more "standard" without it. The fact is, this package is just a collection of headers, so there is nothing to "build" with a make call and it's possible to just skip straight to make install. But should that change for any reason at least our bases are covered this way. Good catch though--it wasn't obvious.

fchapoton commented 5 years ago
comment:4

ok, thanks

fchapoton commented 5 years ago

Reviewer: Frédéric Chapoton

vbraun commented 5 years ago

Changed branch from u/embray/build/destdir-optional-autotools to 1747a4c

embray commented 5 years ago
comment:6

Great to see this merged. Thanks!

embray commented 5 years ago

Changed commit from 1747a4c to none