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.

Sign up for GitHub

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

Jump to bottom

Add default path for GenesisFile and make path relative to configuration yaml file #648

Merged
merged 1 commit into from
Mar 19, 2020
Merged

Add default path for GenesisFile and make path relative to configuration yaml file #648

merged 1 commit into from
Mar 19, 2020

Conversation

Jimbo4350
Copy link
Contributor

@Jimbo4350 Jimbo4350 commented Mar 9, 2020 •
edited
Loading

Issue

Checklist

  • This PR contains all the work required to resolve the linked issue.

  • The work contained has sufficient documentation to describe what it does and how to do it.

  • The work has sufficient tests and/or testing.

  • I have committed clear and descriptive commits. Be considerate as somebody else will have to read these.

  • I have added the appropriate labels to this PR.

@Jimbo4350 Jimbo4350 added the enhancement New feature or request label Mar 9, 2020
@ArturWieczorek
Copy link
Contributor

Can't build it with nix, got an error:

$ nix-build -A scripts.staging.node

[22 of 27] Compiling Cardano.Wallet.Run ( src/Cardano/Wallet/Run.hs, dist/build/Cardano/Wallet/Run.o )
src/Cardano/Wallet/Run.hs:20:1: error: [-Wunused-imports, -Werror=unused-imports]
The import of 'ConfigYamlFilePath'
from module 'Cardano.Config.Types' is redundant
|
20 | import Cardano.Config.Types ( ConfigYamlFilePath(..), NodeConfiguration(..)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^...
...
<< Removed unnecessary output >>
...
builder for '/nix/store/qmj3a0b31g82zisqw2cgsiqh174hkhfh-cardano-node-1.7.0-lib-cardano-node.drv' failed with exit code 1
cannot build derivation '/nix/store/q1b1603l17b5c649qd6bq743pjn65229-cardano-node-1.7.0-exe-cardano-node-config.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/34k1xhcyh0phyqxcq81gkxbpm69a45zw-cardano-node-1.7.0-exe-cardano-node.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/k44iyd9g5h3zclkyzlv0fyc50wchccwj-cardano-node-staging.drv': 1 dependencies couldn't be built
error: build of '/nix/store/k44iyd9g5h3zclkyzlv0fyc50wchccwj-cardano-node-staging.drv' failed

When using cabal to build the error above is marked as a warning:

[22 of 27] Compiling Cardano.Wallet.Run ( src/Cardano/Wallet/Run.hs, /home/artur/Projects/Jordan/cardano-node/dist-newstyle/build/x86_64-linux/ghc-8.6.5/cardano-node-1.7.0/build/Cardano/Wallet/Run.o )

src/Cardano/Wallet/Run.hs:20:1: warning: [-Wunused-imports]
The import of ‘ConfigYamlFilePath’
from module ‘Cardano.Config.Types’ is redundant
|
20 | import Cardano.Config.Types ( ConfigYamlFilePath(..), NodeConfiguration(..)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^...

@Jimbo4350 Jimbo4350 force-pushed the jordan/fix-genesis-file-yaml branch 2 times, most recently from aae9f30 to 4c564eb Compare March 9, 2020 17:12
@rhyslbw rhyslbw mentioned this pull request Mar 11, 2020
Closed
@Jimbo4350 Jimbo4350 force-pushed the jordan/fix-genesis-file-yaml branch 2 times, most recently from e32a54f to 23a1c00 Compare March 18, 2020 09:28
@Jimbo4350
Add default path for GenesisFile if none is specified in the yaml a…
fccd44a 
…nd the `GenesisFile` specified needs to be an absolute path or a relative path, relative to where the config is, not the current working dir
@Jimbo4350 Jimbo4350 force-pushed the jordan/fix-genesis-file-yaml branch from 23a1c00 to fccd44a Compare March 18, 2020 12:08
@Jimbo4350
Copy link
Contributor Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 19, 2020

Build succeeded

@iohk-bors iohk-bors bot merged commit 4498e81 into master Mar 19, 2020
@iohk-bors iohk-bors bot deleted the jordan/fix-genesis-file-yaml branch March 19, 2020 08:20
@disassembler disassembler mentioned this pull request Oct 5, 2020
Merged
dcoutts
dcoutts reviewed Oct 5, 2020
View reviewed changes
cardano-config/src/Cardano/Config/Types.hs
Comment on lines +246 to +258
parseNodeConfigurationFP :: ConfigYamlFilePath -> IO NodeConfiguration
parseNodeConfigurationFP (ConfigYamlFilePath fp) = do
nc <- decodeFileThrow fp
let genFile = unGenesisFile $ ncGenesisFile nc
exists <- doesFileExist genFile
case exists of
-- Genesis file is an absolute path
True -> pure nc
-- Genesis file is a relative path (relative to configuration yaml filepath)
False -> do let d = takeDirectory fp
pure $ nc { ncGenesisFile = GenesisFile $ d </> genFile }

-- TODO: Make genesisfile relative to configuration file as above.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is unnecessary. All we need to do is takeDirectory fp </> genFile. That will make it relative to the config file's dir if the file is relative, but if it's absolute it'll remain unchanged.

There is no need to do this probing to see if files exist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcoutts dcoutts dcoutts left review comments

@ArturWieczorek ArturWieczorek ArturWieczorek approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Jimbo4350 @ArturWieczorek @dcoutts