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

Drop openssl dep #1774

Merged
merged 2 commits into from
Mar 11, 2020
Merged

Drop openssl dep #1774

merged 2 commits into from
Mar 11, 2020

Conversation

kderme
Copy link
Contributor

@kderme kderme commented Mar 10, 2020

With this pr, ouroboros-consensus no longer needs openssl to build. This significantly simplifies building on windows. Related #1082

WIP: TODO nix

@kderme kderme added consensus issues related to ouroboros-consensus Windows WIP Do not merge, work in progress. labels Mar 10, 2020
Copy link
Contributor

@dcoutts dcoutts left a comment

Choose a reason for hiding this comment

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

Assuming those fromIntegral conversions are not needed on any platform then this looks fine.

cabal.project Outdated
@@ -210,7 +210,7 @@ source-repository-package
source-repository-package
type: git
location: https://github.com/input-output-hk/cardano-crypto/
tag: 3c707936ba0a665375acf5bd240dc4b6eaa6c0bc
tag: 2547ad1e80aeabca2899951601079408becbc92c
--sha256: 0g8ln8k8wx4csdv92bz09pr7v9dp4lcyv1334b09c9rgwdwhqg1b
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip:

nix-prefetch-git https://github.com/input-output-hk/cardano-crypto 2547ad1e80aeabca2899951601079408becbc92c
Initialized empty Git repository in /tmp/git-checkout-tmp-mn30nsyu/cardano-crypto-2547ad1/.git/
remote: Enumerating objects: 104, done.
remote: Counting objects: 100% (104/104), done.
remote: Compressing objects: 100% (92/92), done.
remote: Total 104 (delta 5), reused 68 (delta 2), pack-reused 0
Receiving objects: 100% (104/104), 152.66 KiB | 873.00 KiB/s, done.
Resolving deltas: 100% (5/5), done.
From https://github.com/input-output-hk/cardano-crypto
 * branch            HEAD       -> FETCH_HEAD
Switched to a new branch 'fetchgit'
removing `.git'...

git revision is 2547ad1e80aeabca2899951601079408becbc92c
path is /nix/store/bvxsj0yv59b5k3yzsgbl2082i97lc5z8-cardano-crypto-2547ad1
git human-readable version is -- none --
Commit date is 2020-02-18 09:14:29 +0800
hash is 1p2kg2w02q5w1cvqzhfhqmxviy4xrzada3mmb096j2n6hfr20kri
{
  "url": "https://github.com/input-output-hk/cardano-crypto",
  "rev": "2547ad1e80aeabca2899951601079408becbc92c",
  "date": "2020-02-18T09:14:29+08:00",
  "sha256": "1p2kg2w02q5w1cvqzhfhqmxviy4xrzada3mmb096j2n6hfr20kri",
  "fetchSubmodules": false
}

So 1p2kg2w02q5w1cvqzhfhqmxviy4xrzada3mmb096j2n6hfr20kri is the right value here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I should stop abusing ci to give me the correct hash :)

Copy link
Contributor

@mrBliss mrBliss left a comment

Choose a reason for hiding this comment

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

Unrelated to this PR: I actually find it a bit strange that we list a dependency on cardano-crypto in cabal.project but not in stack.yaml 😕

@kderme kderme removed the WIP Do not merge, work in progress. label Mar 10, 2020
@kderme
Copy link
Contributor Author

kderme commented Mar 10, 2020

Unrelated to this PR: I actually find it a bit strange that we list a dependency on cardano-crypto in cabal.project but not in stack.yaml

Hm it's actually specified in the resolver wrapper:
https://raw.githubusercontent.com/input-output-hk/cardano-prelude/3893f21db1665e7bab9e8c6504c547fc7c28f98c/snapshot.yaml

Why do we need this?

@mrBliss
Copy link
Contributor

mrBliss commented Mar 10, 2020

Unrelated to this PR: I actually find it a bit strange that we list a dependency on cardano-crypto in cabal.project but not in stack.yaml

Hm it's actually specified in the resolver wrapper:
https://raw.githubusercontent.com/input-output-hk/cardano-prelude/3893f21db1665e7bab9e8c6504c547fc7c28f98c/snapshot.yaml

Why do we need this?

I guess it's an inconsistency in cardano-prelude. The cardano-crypto dependency is not needed in that project. On the other hand, that snapshot.yaml is used for downstream projects, which means they automatically get this dependency. IMO, we should remove it from snapshot.yaml for consistency.

@mrBliss
Copy link
Contributor

mrBliss commented Mar 10, 2020

@mrBliss
Copy link
Contributor

mrBliss commented Mar 11, 2020

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 11, 2020

@iohk-bors iohk-bors bot merged commit 1a5e8d9 into master Mar 11, 2020
@iohk-bors iohk-bors bot deleted the kderme/windows branch March 11, 2020 08:27
@kderme kderme mentioned this pull request Mar 11, 2020
iohk-bors bot added a commit that referenced this pull request Mar 20, 2020
1789: Fix stack for windows r=mrBliss a=kderme

Related #1082
Continuation of #1774, which makes use of IntersectMBO/cardano-prelude#100

Co-authored-by: kderme <[email protected]>
iohk-bors bot added a commit that referenced this pull request Mar 20, 2020
1789: Fix stack for windows r=mrBliss a=kderme

Related #1082
Continuation of #1774, which makes use of IntersectMBO/cardano-prelude#100

Co-authored-by: kderme <[email protected]>
iohk-bors bot added a commit that referenced this pull request Mar 20, 2020
1789: Fix stack for windows r=mrBliss a=kderme

Related #1082
Continuation of #1774, which makes use of IntersectMBO/cardano-prelude#100

Co-authored-by: kderme <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus issues related to ouroboros-consensus Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants