Skip to content
This repository has been archived by the owner on Aug 2, 2020. It is now read-only.

Feature/wrapper #62

Closed
wants to merge 4 commits into from
Closed

Feature/wrapper #62

wants to merge 4 commits into from

Conversation

angerman
Copy link
Collaborator

This should help with #38, but is not yet ready to be pulled. There are a few issues, I'd appreciate help with:

  • the PackageWrapper is a String for now, which is quite useless. I guess we would need something that can a) query the settings, and b) receive the actual binary name.
  • I have not figured out how to obtain the GhcSourePath in the Rules () to append to prepend to build the absolute path of the executable.

Also this feels like a very crude hack, hence any advice is appreciated.

This adds a very simple "_" suffix to the wrapped binary. (ProgramNameModifier)
Additionally we setup a wrapper target, that depends on the wrapped binary, and
currently does nothing useful except wrapping the binary, without any additional
parameter injection.
@snowleopard
Copy link
Owner

@angerman Thanks for giving it a try! Does the current implementation work for you? I guess it doesn't as I don't see any -B stuff yet.

the PackageWrapper is a String for now, which is quite useless. I guess we would need something that can a) query the settings, and b) receive the actual binary name.

What is the intended purpose of PackageWrapper? Is this the full contents of a wrapper? If yes, this could be done similarly to how Generators work. Have a look at Rules.Generators.ConfigHs.

I have not figured out how to obtain the GhcSourePath in the Rules () to append to prepend to build the absolute path of the executable.

It's not possible, because GhcSourePath cannot escape the Action monad. Why do you need an absolute path in Rules ()?

A couple of further comments:

  • All this wrapper stuff should be disabled on Windows. The relevant condition to check is windowsHost from Oracles.Config.Setting.
  • We need a data type data Program = UnwrappedProgram | WrappedProgram Wrapper or something similar to forbid cases when pkgType == Library and pkgWrapper /= Nothing.

@angerman
Copy link
Collaborator Author

@snowleopard this only works, if I make the follow non-universal adjustments:

        let top = "/Users/angerman/Projects/haskell/ghc/"
        let wrapper = unlines [ "#!/bin/bash"
                              , "exec " ++ top ++ wrappedProgram ++ " -B" ++ top ++ " ${1+\"$@\"}"
                              ]

It's not possible, because GhcSourePath cannot escape the Action monad. Why do you need an absolute path in Rules ()?

I hope this becomes clear from the snipped above.

What is the intended purpose of PackageWrapper? Is this the full contents of a wrapper? If yes, this could be done similarly to how Generators work. Have a look at Rules.Generators.ConfigHs.

Ideally the PackageWrapper should be in the Action monad, such that one can access the whole environment (e.g. read the GhcSourcePath, and potentially other information relevant to the build system). I envision the PackageWrapper to actually build the wrapped files content (e.g. produce what wrapper is above), that would give sufficient flexibility for all potential wrappers, I think.

  • All this wrapper stuff should be disabled on Windows. The relevant condition to check is windowsHost from Oracles.Config.Setting.

Yes, this clearly needs to boil down to a no-op on windows. I'm not sure how to do this elegantly.

  • We need a data type data Program = UnwrappedProgram | WrappedProgram Wrapper or something similar to forbid cases when pkgType == Library and pkgWrapper /= Nothing.

How about the following? Then I'm not so sure what you were debating about cabal package resemblance on IRC, which is why I went ahead with pkgWrapper.

data PackageType = WrappedProgram ProgramWrapper | Program | Library deriving Generic

@snowleopard
Copy link
Owner

I hope this becomes clear from the snipped above.

Hmm, why does the above snippet live in Rules? In Rules we only define targets or match targets, but this snippet builds the target, so it should live in Action where you have access to GhcSourePath and friends.

Ideally the PackageWrapper should be in the Action monad, such that one can access the whole environment (e.g. read the GhcSourcePath, and potentially other information relevant to the build system). I envision the PackageWrapper to actually build the wrapped files content

Exactly! So why don't you turn PackageWrapper into an Expr String? Is this because of a potential import cycle?

Yes, this clearly needs to boil down to a no-op on windows. I'm not sure how to do this elegantly.

OK, don't worry about this. I can add conditionals later.

How about the following?

data PackageType = WrappedProgram ProgramWrapper | Program | Library deriving Generic

I think this is semantically wrong: wrapped and unwapped programs are types of programs, not types of packages. We really should contain the wrapping flavour inside a Program.

Then I'm not so sure what you were debating about cabal package resemblance on IRC, which is why I went ahead with pkgWrapper.

Ah, I see. That was about #12. At some point we might change pkgType from Program | Library to something like pkgPrograms :: [Program] and pkgLibraries :: [Library] to reflect the fact that a single cabal package can mix programs and libraries. This shouldn't influence your work on wrappers.

This allows to access `top`. But we currently hardcode the wrapper for the `ghc` package.
@angerman
Copy link
Collaborator Author

This now correctly generates the binary and wrapper for ghc-stage1. It's still a pretty big hack.

@snowleopard
Copy link
Owner

Awesome!

Let me study your code and see whether it can be simplified if we switch to an alternative non-general solution.

@snowleopard
Copy link
Owner

After experimenting with a couple of ways of adding wrappers I decided to take a non-general approach, see #38 (comment).

Many thanks for this pull request: I used it as a starting point, but merging it wasn't convenient.

@angerman angerman deleted the feature/wrapper branch December 31, 2015 02:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants