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

cli: Use Paths module to get program version #373

Merged
merged 4 commits into from
Jun 6, 2019
Merged

Conversation

rvl
Copy link
Contributor

@rvl rvl commented Jun 6, 2019

The CLIs build normally, but embedFile wasn't working for me in ghci.

  lib/cli/src/Cardano/CLI.hs:275:27: error:
      • Exception when trying to run compile-time code:
          ../../cardano-wallet.cabal: openBinaryFile: does not exist (No such file or directory)
        Code: embedFile "../../cardano-wallet.cabal"
      • In the untyped splice: $(embedFile "../../cardano-wallet.cabal")
      |
  275 |     let cabal = B8.unpack $(embedFile "../../cardano-wallet.cabal")
      |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

There is another easier way to get the program version (assuming all cabal files have the same version).

@rvl rvl requested a review from piotr-iohk June 6, 2019 06:10
@rvl rvl self-assigned this Jun 6, 2019
@jonathanknowles
Copy link
Member

I guess one alternative to this would be to use makeRelativeToProject.

But I think your solution is nicer, as it removes the parsing logic.

@jonathanknowles
Copy link
Member

I tested this out on my system, and the following now works:

$ stack clean
$ stack ghci cardano-wallet-cli:lib

GHCi loads successfully with no errors.

Copy link
Member

@jonathanknowles jonathanknowles left a comment

Choose a reason for hiding this comment

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

This looks like a nice solution. It also uses less code than the alternative, which would be to keep the existing parsing logic and use makeRelativeToProject.

I've tested it out on my system, and didn't run into any issues.

@jonathanknowles
Copy link
Member

@rvl Looks like you have some redundant build-depends entries:

== Section library ==
Redundant build-depends entry
* bytestring
* file-embed
* regex-applicative

@rvl
Copy link
Contributor Author

rvl commented Jun 6, 2019

Thanks.

I would avoid embedFile as much as possible. Also makeRelativeToProject. If it's referring to something above the package tree with ../ then it will often cause build problems.

You generally don't need to repeat what Travis finds because GitHub prevents merging a PR that fails CI, and it can easily be found by clicking on the red cross.

@rvl rvl mentioned this pull request Jun 6, 2019
rvl and others added 3 commits June 6, 2019 09:40
embedFile wasn't working for me in ghci.

  lib/cli/src/Cardano/CLI.hs:275:27: error:
      • Exception when trying to run compile-time code:
          ../../cardano-wallet.cabal: openBinaryFile: does not exist (No such file or directory)
        Code: embedFile "../../cardano-wallet.cabal"
      • In the untyped splice: $(embedFile "../../cardano-wallet.cabal")
      |
  275 |     let cabal = B8.unpack $(embedFile "../../cardano-wallet.cabal")
      |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

There is another easier way to get the program version (assuming all
cabal files have the same version).
@KtorZ
Copy link
Member

KtorZ commented Jun 6, 2019

I've also modified the hard-coded version from the integration test scenario to use this approach instead. Neat 👍

@KtorZ KtorZ merged commit 24eb7a7 into master Jun 6, 2019
@KtorZ KtorZ deleted the rvl/cli-version branch June 7, 2019 09:06
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.

None yet

5 participants