polkadot-sdk icon indicating copy to clipboard operation
polkadot-sdk copied to clipboard

Clean up `git` dependencies

Open ggwpez opened this issue 1 year ago • 20 comments

### Tasks
- [x] [simple-mermaid](https://github.com/kianenigma/simple-mermaid) used [here](https://github.com/paritytech/polkadot-sdk/blob/b51904da5681c479e462ab0b5bdd7464dfecd27a/docs/sdk/Cargo.toml#L25)
- [x] [ethabi-decode](https://github.com/snowfork/ethabi-decode) used [here](https://github.com/paritytech/polkadot-sdk/blob/127b9bec15f8a83964ef6049209df75da47b1479/bridges/snowbridge/parachain/pallets/system/Cargo.toml#L36)
- [ ] [bandersnatch_vrfs](https://github.com/w3f/ring-vrf) used [here](https://github.com/paritytech/substrate/blob/e966352d118a745678f720ae85e617d054dc8165/primitives/core/Cargo.toml#L62)
- [x] [milagro_bls](https://github.com/snowfork/milagro_bls) from [here](https://github.com/paritytech/polkadot-sdk/blob/bab0348372b58d5106333faa597423242e831a31/bridges/snowbridge/parachain/primitives/beacon/Cargo.toml#L34)

ggwpez avatar Jan 13 '24 14:01 ggwpez

For bandernatch (cc @swasilyev):

  • [ ] Publish https://github.com/w3f/fflonk
  • [ ] Publish https://github.com/w3f/ring-proof
  • [ ] Introduce ark-ec-vrfs

davxy avatar Jan 13 '24 16:01 davxy

there is also an issue for ethabi-decode here: https://github.com/paritytech/parity-bridges-common/issues/2775 (afaik, Vincent will work on it)

cc: @vgeddes @claravanstaden

bkontur avatar Jan 13 '24 22:01 bkontur

Ack, yes, I'll publish ethabi-decode and milagro-bls this week. Thanks for the reminder.

vgeddes avatar Jan 15 '24 15:01 vgeddes

I've published them:

  • https://crates.io/crates/ethabi-decode
  • https://crates.io/crates/snowbridge-milagro-bls

We will update our cumulus/bridges/snowbridge subtree next week which will include these changes.

vgeddes avatar Jan 19 '24 15:01 vgeddes

From a crates perspective. Our tooling with auto strip optional git depends or switch git dependencies to crates.io if a compatible version exists. Meanwhile git deps with no crates.io releases are totally unpublishable. So releasing milagro_bls is high priority to me as it made some crates unpublishable on the last release.

Morganamilo avatar Jan 25 '24 12:01 Morganamilo

@Morganamilo thanks for the info. As @vgeddes mentioned, the crates are published and I have updated their usages in this PR: https://github.com/paritytech/polkadot-sdk/pull/3029

claravanstaden avatar Jan 25 '24 13:01 claravanstaden

@Morganamilo Which crates would still blocked after #3029 and by what?

ggwpez avatar Jan 25 '24 14:01 ggwpez

That was the only crate I had issue with. Checking again, it was named milagro_bls on my end but I only see snowbridge-milagro-bls on crates.io. I guess the crate was renamed but that didn't make the release window and I got the old name?

Morganamilo avatar Jan 25 '24 23:01 Morganamilo

Exactly, @Morganamilo.

claravanstaden avatar Jan 26 '24 07:01 claravanstaden

Hey @Morganamilo! I was expecting the snowbridge crates to be updated with the polkadot-sdk 1.7.0 release, but it looks like the crates are still empty: https://crates.io/crates/snowbridge-pallet-inbound-queue/0.0.0

Will you let know if there is an issue with the publishing, or any other problem?

claravanstaden avatar Feb 07 '24 11:02 claravanstaden

I think the crate publishing takes a while - we are also rate limited by crates-io. You can check here if stuff is being published https://crates.io/users/parity-crate-owner?sort=recent-updates

ggwpez avatar Feb 08 '24 14:02 ggwpez

Is there anything blocking simple mermaid? Would moving to 0.1.1 work or do we need features that are unreleased?

Morganamilo avatar Feb 13 '24 10:02 Morganamilo

Yea 0.1.1 should include my fix.
I will try to remove it here in the repo. PS: https://github.com/paritytech/polkadot-sdk/pull/3304

ggwpez avatar Feb 13 '24 10:02 ggwpez

Done now @Morganamilo

ggwpez avatar Feb 14 '24 23:02 ggwpez

We require some CI check that prevents adding git dependencies for crates that are already released.

milagro_bly from here

We should not care about this stuff. We should not release external crates.

bkchr avatar Mar 11 '24 10:03 bkchr

We should not care about this stuff. We should not release external crates.

WDYM? Having git dependencies is bad for our tooling. So they should be banned in general.
Ah nvm. with "crates that are already released" you probably meant our crates, not external ones 👍

ggwpez avatar Mar 11 '24 11:03 ggwpez

WDYM? Having git dependencies is bad for our tooling. So they should be banned in general.

I meant that the linked crate belongs to snowbridge. We as Parity should not release snowbride crates.

bkchr avatar Mar 11 '24 12:03 bkchr

I meant that the linked crate belongs to snowbridge. We as Parity should not release snowbride crates.

Yes sure. That is why snowfork team is help us with this. Thanks Vincent and Clara.

ggwpez avatar Mar 11 '24 13:03 ggwpez

We published that crates, yes. It will remain the Snowbridge team's responsibility.

claravanstaden avatar Mar 11 '24 13:03 claravanstaden

It looks like litep2p is now being pulled in. @lexnv do you think it would be possible to use crates-io releases?
We can otherwise not properly release the sc-network crates that depend on it.

ggwpez avatar May 15 '24 12:05 ggwpez

Going to close this since the remaining bandersnatch dep is feature gated and there not blocking the release process.
A CI check is also in place to prevent the addition of new git deps.

ggwpez avatar Jun 27 '24 13:06 ggwpez