flatsurf / e-antic

Embedded algebraic number fields
https://flatsurf.github.io/e-antic/libeantic/
GNU Lesser General Public License v3.0
12 stars 11 forks source link

render the local header systemwise #235

Open jgmbenoit opened 2 years ago

jgmbenoit commented 2 years ago

This patch renders the local header local.h systemwise. Note that this header should be actually installed in an architecture dependent include hierarchy.

codecov[bot] commented 2 years ago

Codecov Report

Merging #235 (98f2766) into master (acad331) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #235   +/-   ##
=======================================
  Coverage   92.00%   92.00%           
=======================================
  Files         105      105           
  Lines        2075     2075           
=======================================
  Hits         1909     1909           
  Misses        166      166           
Impacted Files Coverage Δ
libeantic/e-antic/renf_elem.h 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update acad331...98f2766. Read the comment docs.

saraedum commented 2 years ago

Thanks for the PR @jgmbenoit.

Note that this header should be actually installed in an architecture dependent include hierarchy.

Where would such headers go typically? I only know about includedir which is "The directory for installing C header files" according to the autoconf manual. Isn't that architecture dependent already typically?

jgmbenoit commented 2 years ago

On Debian multi-arch boxes (e.g., i386 (32bits) + amd64 (64bits)) the hierarchy /usr/include is shared, so it is architecture independent. Debian puts architecture dependent header under the hierarchy /usr/include/<triplet> as it puts libraries under /usr/lib/<triplet>. My current understanding is that we have to set it by hand when we use autotools. This page might be a good start. For Debian, I am applying a Debian specific patch so that the <triplet> can be passed at building time (at the time of writing, the git repository used by Debian is down, so I cannot provide a link to the patch) .
Otherwise, local.h is a very generic name for an header: it would be a very good idea to rename it to avoid any name collision. Something as e-antic-local.h would make the trick.

saraedum commented 2 years ago

On Debian multi-arch boxes (e.g., i386 (32bits) + amd64 (64bits)) the hierarchy /usr/include is shared […]

I see. Thanks for the explanation.

Otherwise, local.h is a very generic name for an header: it would be a very good idea to rename it to avoid any name collision. Something as e-antic-local.h would make the trick.

Wouldn't it always be installed into an e-antic/ subdirectory? Even if it it's architecture dependent?

jgmbenoit commented 2 years ago

Wouldn't it always be installed into an e-antic/ subdirectory? Even if it it's architecture dependent?

Indeed. Having said that, a "ceinture et bretelles" ("belt and braces") approach cannot hurt here.

jgmbenoit commented 2 years ago

is this patch applied in version 1.2.0 ?

saraedum commented 2 years ago

is this patch applied in version 1.2.0 ?

No. I think there were still some open discussions here that we were waiting for.