#374 Ada guidelines changes for Comfignat and runpaths
Closed: Fixed None Opened 5 years ago by rombobeorn.

Comfignat is a new build system built around the GNAT tools and designed to make packaging easy. The RPM macro Comfignat_make handles building a Comfignat-using package with the right configuration for Fedora. This macro needs to be mentioned in the Ada guidelines. I propose this change:

https://fedoraproject.org/w/index.php?title=User:Rombobeorn/Ada_Guidelines_Changes_2&diff=361215&oldid=361212

A macro named GNAT_add_rpath can be defined in spec files to allow a runpath to be added, for example in test suites or auxiliary programs that aren't installed but run during the build. This should also be explained in the Ada guidelines. I propose this addition:

https://fedoraproject.org/w/index.php?title=User:Rombobeorn/Ada_Guidelines_Changes_2&diff=361216&oldid=361215

Here's the complete proposed new version of the Ada guidelines:

https://fedoraproject.org/wiki/User:Rombobeorn/Ada_Guidelines_Changes_2


A macro named GNAT_add_rpath can be defined in spec files to allow a runpath to be added, for example in test suites or auxiliary programs that aren't installed but run during the build.

The big problem here was that the default position of most (if not all) of the FPC is "rpath is really bad, do not approve without a really good reason". I'm willing to believe that there might be a really good reason for ada %check builds, but you didn't make it.

What you want to do is redo the wording of the proposed change, and/or provide extra text explaining:

  1. Why is using rpath significantly better than using LD_LIBRARY_PATH (or something else). Basically what is the benefit of allowing it.

  2. How will ada packaging ensure that programs built using rpath don't get installed. Basically can we ensure the cost is small.

FPC voted on the two parts of the Ada guidelines change separately last week. One change passed and the other failed:

info Add information about Comfignat build tool to the Ada Guidelines Passed (+1:5, 0:0, -1:0)

info Add explanation of the GNAT_add_rpath macro to the Ada guidelines did not pass. Feedback given in ticket (+1:0, 0:4, -1:0)

(See https://fedorahosted.org/fpc/ticket/374#comment:2 for feedback on this failure and posible ways you could make it acceptable)

I reordered where in the list the comfignat section was located as noted in the meeting. It's now been added. Announcement text:

"""

Instructions on using comfignat to build ada packages has been added to the Ada Guidelines:
https://fedoraproject.org/wiki/Packaging:Ada

"""

I had planned to attend the meeting but when this wasn't on the agenda I thought I could spend my time elsewhere.

Maybe I should provide some background for those who aren't familiar with the GNAT tools.
Adacore's customers want relocatable binary packages that they can put in arbitrary places in their filesystem without recompiling, and therefore Gnatmake and GPRbuild both add a runpath by default. Since we don't want that in Fedora our RPM macros contain the option -R to disable the runpath.

When the tools do something automatically, people tend to rely on it. In other cases where testcases or helper programs run during the build and need to load a just built library, I suppose the developers code the makefiles to provide the path, either as a runpath or with LD_LIBRARY_PATH or maybe with Libtool. Developers who use the GNAT builders don't need to do that because the builder handles it automatically. But then we pass -R and it breaks these use cases.

This wouldn't be a problem if the builders had a "-no-R" option that could be passed to negate -R and restore the default behaviour. But there is no such option and so I came up with a way to avoid passing -R in selected cases.

And now to the questions:

  1. Why is using rpath significantly better than using LD_LIBRARY_PATH (or something else). Basically what is the benefit of allowing it.

Disabling the automation and replacing it with a hardcoded path in the spec file can increase the maintenance burden and introduces an additional point where things can potentially go wrong. The path in the spec may need to be adjusted when the package is upgraded to a new version, and if the packager gets it wrong, then test cases or helper programs might silently fail to find the library that was just built and instead load an older version that happens to be installed.

From the meeting log:

17:35:43 <limburgher> I'm always dubious of %check running on something other than what's installed. It's either pointless or could generate false positives or negatives.

That would indeed be bad, and that's an argument for allowing an automatic runpath in test cases to ensure that they really do test the actual library that was just built and not some other version.

Building the library a second time with different options would of course be a bug.

  1. How will ada packaging ensure that programs built using rpath don't get installed. Basically can we ensure the cost is small.

The best way to ensure that would seem to be to use /usr/lib/rpm/check-rpaths in Koji – for all packages, not just Ada.

Barring that, what we can do is to write policy. The Ada gudelines page already states that Ada packages must follow the general packaging guidelines, and that includes the section "Beware of Rpath". I suppose I could stress that once more in the paragraph about GNAT_add_rpath, if you think that would make people take notice better. I could add the sentence "This macro must not be defined in those parts of the spec file where programs or libraries to be installed are built, and does not in any way exempt the package from what the Packaging Guidelines say about runpaths.", and link directly to Packaging:Guidelines#Beware_of_Rpath.

Honestly, you have more reason to worry that the optflags won't be applied. That also causes a runpath to be included as the defaults are then in effect. I have personally found several such mistakes in reviews and in existent Ada packages.

As was decided in the latest meeting, I have tested running %{_rpmconfigdir}/check-rpaths as the last thing in %check in several packages, and built them with Mock or plain RPMbuild, and even some Koji scratch-builds. It worked as expected in all cases: It caught various kinds of errors that introduced runpaths in installed programs or libraries, and did not cause any trouble when there was no runpath or when runpaths were only used in helper programs or testcases.

The risk of accidental runpaths is fairly high in Ada packages for several concurrent reasons, so I think check-rpaths should be run in all Ada packages unless it turns out to cause problems. I suppose there must be some reason why running check-rpaths isn't required of all packages, so I propose to make it mandatory only for packages that use GNAT_add_rpath, and recommended for other Ada packages.

No addition to !BuildRequires is needed. check-rpaths is packaged together with RPMbuild, which is of course always installed when a package is built. The package, rpm-build, is also in the list of packages that don't need to be specified as build-time dependencies.

Here is the updated proposal:

https://fedoraproject.org/w/index.php?title=User%3ARombobeorn%2FAda_Guidelines_Changes_2&diff=365584&oldid=365581

info Latest GNAT_add_rpath changes approved (+1:6, 0:0, -1:1)

Pavel Zhukov will talk at FOSDEM about Ada in Fedora. It would be nice if our Ada policy page would by then reflect the current decided policy, in case someone in the audience gets interested.

Sorry for taking so long to get back to this. Guideline is now updated to include the runpath section. New announcement text:

"""

Two changes have been made to the Ada Guidelines:
instructions on using comfignat to build ada packages has been added to the Ada Guidelines:
instructions on how to allow rpaths for testsuites and binaries that are only used at buildtime. The instructions also include a requirement to explicitly run %{_rpmconfigdir}/check-rpaths in %check if rpaths are turned on for the build.

https://fedoraproject.org/wiki/Packaging:Ada

"""

Login to comment on this ticket.

Metadata