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

Import cardano addresses repo #492

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Conversation

palas
Copy link

@palas palas commented Aug 30, 2024

This PR integrates the repo https://github.com/IntersectMBO/cardano-addresses in https://github.com/IntersectMBO/cardano-base.

This PR:

  • Moves the three haskell projects from cardano-addresess repo (the pattern is using the name of the cabal file/project as folder inside):
    • core -> cardano-addresses/cardano-addresses
    • command-line -> cardano-addresses/cardano-addresses-cli
    • jsapi -> cardano-addresses/cardano-addresses-jsapi
  • Also the mainly JS project:
    • jsbits -> cardano-addresses/cardano-addresses-jsbits
  • Adds a nix output that generates a docker image with the cardano-cli executable (e.g: .#docker.x86_64-linux.cardano-address).
  • Adds ghcjs cross-compilation
    • But I have discarded all packages from ghcjs cross-compilation except for those in cardano-addresses, because they weren't compiling.
  • Migrates the three custom derivations from cardano-addresses-js:
    • .#cardano-addresses-js
    • .#cardano-addresses-demo-js
    • .#cardano-addresses-js-shell
  • Adds convenience derivation with patched sources of cardano-addresses-jsbits called .#jsbits, because it seemed quite necessary for following the README.mds in jsapi and jsbits.
  • Does some fixes to the cardano-addresses code to make it compatible with ghc above 9.8.2, some required having some CPP directives.

Considerations:

  • jsapi tests are disabled except for ghcjs, I think they are only meant to work on JS
  • I've tried to adapt the README.mds, and I moved the one in the root to the cardano-addresses subdir.
  • I haven't migrated the musl outputs, or any artifact generation. Happy to do them in another PR (or even partially in this one).
  • There is an issue with docs, in cardano-addresses the haddock is not in the top level index because there are more things deployed: demo app and js doc. I haven't migrated either, it would require to move the cardano-base a level higher. Happy to do that in another PR.
  • Dependency on ghcjs-overlay, needs to be updated periodically for cabal build to work without nix, this is done with an action currently. This is important for the CI action that caches the CodeSpaces. Don't know how the CI action is triggered, may need credentials.
  • There is an associated NPM package, and it is still pointing to the input-output-hk repo for documentation. Presumably that requires credentials to manage too.
  • In order to use the cardano-addresses libraries from this repo, I think it is necessary to cut a new version of the library and add it to CHaP. I have never done a CHaP release, but probably makes sense to do it in a separate PR.
  • I have disabled hjsonschema library for versions of ghc greater than 9.8.2, because it doesn't compile and is deprecated. It seems this only affects a test which is still being run on the other versions of ghc, so it is not a problem yet. Also, it was already disabled for ghcjs.

@palas palas self-assigned this Aug 30, 2024
@palas palas force-pushed the import-cardano-addresses-repo branch 2 times, most recently from 379e81b to 1c6b3e4 Compare September 2, 2024 18:33
@palas palas marked this pull request as ready for review September 2, 2024 18:57
@palas palas marked this pull request as draft September 2, 2024 18:57
@palas palas force-pushed the import-cardano-addresses-repo branch 16 times, most recently from aa66568 to 6a44437 Compare September 5, 2024 19:10
@palas palas marked this pull request as ready for review September 5, 2024 20:41
@palas palas requested a review from Jimbo4350 September 5, 2024 20:43
@palas palas force-pushed the import-cardano-addresses-repo branch from 70ddc39 to 6a44437 Compare September 6, 2024 18:48
@palas palas requested a review from a team as a code owner September 13, 2024 01:40
@palas palas force-pushed the import-cardano-addresses-repo branch from f0c7d10 to 6d867d6 Compare September 13, 2024 21:29
@palas
Copy link
Author

palas commented Sep 13, 2024

@palas
Copy link
Author

palas commented Sep 13, 2024

Commit: aa66568

Copy link
Collaborator

@lehins lehins left a comment

Choose a reason for hiding this comment

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

Oh my god! 😮 +42K lines of code. cardano-addresses is bigger than the whole of cardano-base repo right now.

We need to seriously discuss the future of cardano-addresses, because I am no longer sure that embedding it into any other repo is a good idea.
Looking at all of the changes in this PR and the massive size of cardano-addresses makes me realize that this is just not going to 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.

2 participants