No support for `wrapper` about hadrian HOT 14 CLOSED

It is a mystery for me why everything works on Windows without wrappers.

Can we get rid of wrappers? They seem to be just an unnecessary layer for massaging argument lists passed to a builder like hsc2hs. We can certainly achieve that in the new build system without any wrappers.

from hadrian.

Maybe @bgamari or @quchen can shed some light on the old build systems need for wrappers, might know who would know?

from hadrian.

I would guess that the wrappers are necessary as we dynamically link on platforms other than Windows, meaning that we must set LD_LIBRARY_PATH to ensure that the dynamic linker can find the needed dynamic libraries.

from hadrian.

There seem to be the following wrapper scripts only in the ghc tree:

$ find . -name "*.wrapper"                                                                                                 
./ghc/ghc.wrapper
./utils/ghc-pkg/ghc-pkg.wrapper
./utils/haddock/haddock.wrapper
./utils/hpc/hpc.wrapper
./utils/hsc2hs/hsc2hs.wrapper
./utils/runghc/runghc.wrapper

I guess it boils down to figuring out if the build system initiates all invocations, or if other programs invoke the wrapped programs in question. If it's only the former case, shake could take care of providing the LD_LIBRARY_PATH, in the latter, we'll need them anyway.

from hadrian.

There is rules/shell-wrapper.mk which does a lot of things that can be moved to the new build system.

It also manipulates LD_LIBRARY_PATH, with help from rules/library-path.mk, but it seems strange that we can't deal with this without wrappers.

from hadrian.

Keep in mind that you want to retain the ability to call these utilities outside of the build system. One way to accomplish this is to link executables with -rpath however I believe there may be portability issues here.

from hadrian.

@bgamari Yes, I see. We can at least minimise the amount of stuff that goes into wrappers. If we keep them only for passing LD_LIBRARY_PATH then they will be a lot easier to understand, maintain, and potentially remove.

I guess this issue is a prerequisite for #4.

from hadrian.

The idea I mentioned on IRC was using environment vars. Assuming we export GHC_TOP_DIR, and change ghc to look for GHC_TOP_DIR in the environments if -B is not found.
Similarly provide the default values for other default arguments the wrapper supply. This though requires to adapt all the programs that are called through wrappers. This also requires that environment variables are properly handed down to invoked commands, which I'm not sure all do.

My current view is that we need wrapper support in the shake build system until we can optimize it out, if ever. As others have mentioned, wrapper are quite common on linux.

from hadrian.

I'll try to give this a go. Fingers crossed.

from hadrian.

I'd like to document an alternative idea to implementing wrappers here.

Instead of a general solution (e.g., #62), which extends the Program type to something like WrappedProgram Wrapper | UnwrappedProgram, we could consider a custom solution similar to includes generated files. In this solution we simply create 5 custom build rules for wrappers and need these wrappers when building corresponding builders.
Cons:

• Not general.

Pros:

• Simpler to implement, and possibly to understand too.
• Does not pollute Package datatype with wrapper-related stuff, hence having less impact on the size of Shake database and performance.

from hadrian.

@snowleopard for that, we would still need to conditionally place the binaries for the wrapped packages into other locations, which I can not work out so far if the package doesn't know if it's wrapped or not :-/

from hadrian.

@angerman I think we can handle this nicely in Rules.Program: we add special rules (with priority 2.0) for wrapped programs that produce wrappers instead of binaries, and also put actual binaries into inplace/lib directory.

from hadrian.

After experimenting with @angerman's solution, I reworked it into a less disruptive one, see a1eab18.

No changes to top-level definitions of packages are now required, everything is handled locally in Rules/Program.hs with wrappers stored in Rules/Wrappers directory. I like the fact that wrappers are now well-hidden: seeing them right in GHC.hs or Package.hs could be confusing, especially for newcomers.

To add a new wrapper one needs to add a new file into Rules/Wrappers and add a pair (PartialTarget, Wrapper) to the list of wrappers in Rules/Program.hs.

@angerman Could you please test this solution and add missing wrappers if required?

from hadrian.

Looks like the solution we now have for wrappers works sufficiently for now. I'm certain other issues will come up, and there may be debates on the design of the wrapper logic, we should open new issues for those.

from hadrian.