Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add conversion from cabal files #114

Closed

Conversation

yamadapc
Copy link

@yamadapc yamadapc commented Aug 2, 2016

Adds modules Hpack.Convert and Hpack.Convert.Run for reading a
GenericPackageDescription onto a Package and running the user-facing
conversion routine.

The user-facing option is exposed through:

$ hpack --convert [cabal-file|project-dir]

Also adds tests testing we can read cabal files generated from some of
the README examples and hpack's into package.yamls that look like the
original ones.

Everything should be supported, including converting conditionals.

I really need feedback on how to make Hpack.Config Package related
ToJSON instances cleaner. That's the ugly bit of the code.

There some cases where we run into problems in conversion. For example,
the Cabal library does not expose in its data-structures if buildable
was specified or not. To keep the output clean, I initially striped it
from sections if it was true, but then empty conditional blocks could be
generated. Right now it'll only output buildable if it's not
redundant based on context. It'll also remove any empty conditional
branches.

This closes #63.

Adds modules `Hpack.Convert` and `Hpack.Convert.Run` for reading a
`GenericPackageDescription` onto a `Package` and running the user-facing
conversion routine.

The user-facing option is exposed through:
```
$ hpack --convert [cabal-file|project-dir]
```

- - -

Also adds tests testing we can read cabal files generated from some of
the README examples and hpack's into package.yamls that look like the
original ones.

Everything should be supported, including converting conditionals.

I really need feedback on how to make `Hpack.Config` `Package` related
`ToJSON` instances cleaner. That's the ugly bit of the code.

There some cases where we run into problems in conversion. For example,
the Cabal library does not expose in its data-structures if `buildable`
was specified or not. To keep the output clean, I initially striped it
from sections if it was true, but then empty conditional blocks could be
generated. Right now it'll only output `buildable` if it's not
redundant based on context. It'll also remove any empty conditional
branches.

This closes sol#63.
@yamadapc
Copy link
Author

yamadapc commented Aug 2, 2016

Tests are failing because of a key sorting and an encoding issue. I'll fix later today.

@mgsloan
Copy link
Contributor

mgsloan commented Aug 3, 2016

Awesome!!

dropIfGH v = v
in
HashMap.insert "github" (String (T.pack (drop (length githubBaseUrl) srepo))) $
HashMap.alter dropIfGH "bug-reports" $
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's me, I would try to avoid this kind of logic here and instead do things like this in a conversion step.

@sol
Copy link
Owner

sol commented Aug 4, 2016

Hey, this looks really useful. But it's quite some code and my bandwidth is still limited. @yamadapc would you be willing to maintain this as a separate package?

We would then add prominent references to the README etc., so that discoverability is not an issue.

I think even if I would do it myself, I would at least go for two separate executables (UNIX style).

@yamadapc
Copy link
Author

yamadapc commented Aug 4, 2016

I'm fine with doing that.

I started it and ran into a couple of problems:

  • I need to expose HasFieldNames from Hpack.Config
  • I need to expose Hpack.GenericsUtil and Hpack.Util

Those are the ones I can solve with a fork/PR, no biggie.


And then there's one which I can't understand:

        Couldn't match type ‘Rep SourceRepository’
                       with ‘GHC.Generics.M1 GHC.Generics.D d1 m1’
        The type variables ‘d1’, ‘m1’ are ambiguous
        In the expression: genericToJSON_
        In an equation for ‘toJSON’: toJSON = genericToJSON_
        In the instance declaration for ‘ToJSON SourceRepository’

In all uses of the genericToJSON_ function. I think it has to do with the typeName stuff only working for data types defined in the current module. I'm not familiar with GHC.Generics.

Maybe it's what these guys are talking about..

Ideas?

@soenkehahn
Copy link
Collaborator

@yamadapc: Do you have a wip branch where we can take a look at the code producing that Generics type error?

@yamadapc
Copy link
Author

yamadapc commented Aug 8, 2016

@soenkehahn This gives you the error: https://github.com/yamadapc/hpack-convert-separate-package-with-hpack-dependency

But talking with Simon, we had reached the conclusion that it'd be easier to leave hpack-convert on a separate package that'd just fork hpack. I just need to do the laundry on this enhancement/63-convert-cabal-files branch and update the executable/README.

EDIT A gist of the full output https://gist.github.com/9479f0bd3843e81a20ebfbd771614d1d

@soenkehahn
Copy link
Collaborator

@yamadapc: I see. Looking forward to this. :)

@yamadapc
Copy link
Author

yamadapc commented Aug 9, 2016

Ok, it has been published on Hackage as hpack-convert and is on my GitHub account as an hpack fork in https://github.com/yamadapc/hpack-convert.

People can choose to:

I'll keep it on my todo list to make it clean and have it merged back upstream. Even though it's a fork it's not hard to just cherry-pick commits back here at any point.


Closing this in favor of this approach.

@yamadapc yamadapc closed this Aug 9, 2016
@sol
Copy link
Owner

sol commented Aug 10, 2016

We may still want to add it to the README. PRs welcome!

Sent from mobile

On 9 Aug 2016, at 6:55 PM, Pedro Tacla Yamada [email protected] wrote:

Closed #114.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

@soenkehahn
Copy link
Collaborator

@yamadapc: Just tried this out, awesome work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatic generation of hpack from .cabal files?
4 participants