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

remove iogx #122

Merged
merged 5 commits into from
Oct 1, 2024
Merged

remove iogx #122

merged 5 commits into from
Oct 1, 2024

Conversation

tgunnoe
Copy link
Member

@tgunnoe tgunnoe commented Sep 17, 2024

ETCM-8233

iogx is unnecessarily complex for our use cases. nosys essentially only handles systems for you, rewriting the outputs. As such, it also fixes interfaces like 'nix flake show' where Input From Derivation (haskell.nix) would usually break the interface.

Other changes:

  • Shells made into a single shell
  • Updated nixci to omnix
  • Cleanup outputs

Prereview checklist

  • The CHANGELOG.md has been updated under the # Unreleased header using the appropriate sub-headings with links to the appropriate issues/PRs.
  • All tests pass in CI
  • PR was self-reviewed

@tgunnoe tgunnoe force-pushed the feat/remove-iogx branch 3 times, most recently from 3c640cc to 1fae27c Compare September 18, 2024 16:51
@tgunnoe tgunnoe changed the title wip: remove iogx remove iogx Sep 18, 2024
@tgunnoe tgunnoe marked this pull request as ready for review September 18, 2024 16:52
@tgunnoe tgunnoe force-pushed the feat/remove-iogx branch 8 times, most recently from 706c444 to 4e4c2eb Compare September 19, 2024 12:49
};
};

upToDatePlutusScriptCheck = pkgs.runCommand "up-to-date-plutus-scripts-check"
Copy link
Contributor

Choose a reason for hiding this comment

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

might be nice to have this in a seperate script, perhaps somewhere in nix/scripts?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. This is just a migration from how it was before (and legacy stuff from mlabs). And there's probably a non-nix way within the makefiles somewhere as well, so I'll just create a ticket to keep track of it.
In general, let scripts be useable outside of nix.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah I saw it wasn't any better before the change, just nice to split up longer than a few lines to its own file

flake.nix Show resolved Hide resolved
nix/outputs.nix Outdated Show resolved Hide resolved
nix/outputs.nix Outdated Show resolved Hide resolved
@kpinter-iohk
Copy link
Contributor

Nix shell doesn't seem to provide cabal. Not sure if I'm missing something.

$ nix develop
$ cabal
bash: cabal: command not found

@tgunnoe
Copy link
Member Author

tgunnoe commented Sep 19, 2024

Nix shell doesn't seem to provide cabal. Not sure if I'm missing something.

$ nix develop
$ cabal
bash: cabal: command not found

Thanks, looks like iogx provided that while it made its shells. I'll add the toolchains missing from there

nix/shells.nix Outdated Show resolved Hide resolved
nix/shells.nix Outdated Show resolved Hide resolved
@tgunnoe tgunnoe force-pushed the feat/remove-iogx branch 2 times, most recently from c71aee0 to d7dc6de Compare September 20, 2024 01:22
@kpinter-iohk
Copy link
Contributor

kpinter-iohk commented Sep 20, 2024

After pulling:

  1. cd offchain/ && make update-scripts fails to generate RawScripts.purs. This is because for some reason IO.withFile doesn't handle Unicode anymore, and there are and characters in the output it's supposed to write. Seems like there was some magic in the previous version that fixed this. I'm okay with replacing these characters with their non-unicode equivalents too (> and ::).
  2. After fixing the above issue with the use of non-Unicode chars, make build fails with this again: remove ctl builds and build everything using local tooling #98 (comment)

Minor comment:
I also noticed that on master we have cabal 3.12.1.0 and ghc 9.6.6, while on this we have cabal 3.10.3.0 and ghc 9.6.5.

@tgunnoe
Copy link
Member Author

tgunnoe commented Sep 20, 2024

Thanks Krisztian. yeah, the tooling isn't correct yet. there was a whole chain of tooling that iogx would provide locally. I'm still working on it and will re-request your review once I get it right.

@tgunnoe
Copy link
Member Author

tgunnoe commented Sep 23, 2024

@kpinter-iohk can you to see if all of your shell/build tooling is functional now?

nix/shells.nix Outdated Show resolved Hide resolved
tgunnoe and others added 4 commits September 26, 2024 01:33
ETCM-8233

iogx is unnecessarily complex for our use cases. nosys essentially only handles
systems for you, rewriting the outputs. As such, it also fixes interfaces lik  e
'nix flake show' where Input From Derivation (haskell.nix) would usually break
the interface.

Other changes:

- Shells made into a single shell
- Updated nixci to omnix
- Cleanup outputs
@kpinter-iohk
Copy link
Contributor

I see that the CI broke because script sizes changed. I wanted to look into this locally, however cabal build now fails when trying to resolve dependencies:

$ cabal build --minimize-conflict-set
Warning: Requested index-state 2024-07-16T05:49:34Z is newer than
'hackage.haskell.org'! Falling back to older state (2024-07-16T05:41:59Z).
Resolving dependencies...
Error: cabal: Could not resolve dependencies:
[__0] trying: trustless-sidechain-6.1.0 (user goal)
[__1] next goal: plutus-tx (dependency of trustless-sidechain)
[__1] rejecting: plutus-tx-1.30.0.0, plutus-tx-1.29.0.0, plutus-tx-1.28.0.0
(constraint from project config
/home/.../partner-chains-smart-contracts/onchain/cabal.project requires
^>=1.27)
[__1] trying: plutus-tx-1.27.0.0
[__2] next goal: plutus-core (dependency of trustless-sidechain)
[__2] rejecting: plutus-core-1.30.0.0 (conflict: plutus-tx =>
plutus-core^>=1.27)
[__2] skipping: plutus-core-1.29.0.0, plutus-core-1.28.0.0 (has the same
characteristics that caused the previous version to fail: excluded by
constraint '^>=1.27' from 'plutus-tx')
[__2] trying: plutus-core-1.27.0.0
[__3] next goal: cardano-crypto-class (dependency of trustless-sidechain)
[__3] rejecting: cardano-crypto-class-2.1.5.0, cardano-crypto-class-2.1.4.0,
cardano-crypto-class-2.1.3.0, cardano-crypto-class-2.1.2.0 (conflict:
pkg-config package libblst-any, not found in the pkg-config database)
[__3] rejecting: cardano-crypto-class-2.1.1.0 (conflict: plutus-core =>
cardano-crypto-class^>=2.1.2)
[__3] skipping: cardano-crypto-class-2.1.0.2, cardano-crypto-class-2.1.0.1,
cardano-crypto-class-2.1.0.0, cardano-crypto-class-2.0.0.1,
cardano-crypto-class-2.0.0.0.1, cardano-crypto-class-2.0.0 (has the same
characteristics that caused the previous version to fail: excluded by
constraint '^>=2.1.2' from 'plutus-core')
[__3] fail (backjumping, conflict set: cardano-crypto-class, plutus-core,
trustless-sidechain)
After searching the rest of the dependency tree exhaustively, these were the
goals I've had most trouble fulfilling: plutus-tx, plutus-core,
cardano-crypto-class, trustless-sidechain

@tgunnoe tgunnoe force-pushed the feat/remove-iogx branch 2 times, most recently from 21dfc1c to 0eeb2c2 Compare September 30, 2024 15:07
Copy link
Contributor

@kpinter-iohk kpinter-iohk left a comment

Choose a reason for hiding this comment

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

🎉 🥇

@tgunnoe tgunnoe merged commit c048272 into master Oct 1, 2024
4 checks passed
@tgunnoe tgunnoe deleted the feat/remove-iogx branch October 1, 2024 12:12
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.

4 participants