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

fix: remove invalid unpacking logic for IPFS pinned pkg sources #6902

Merged
merged 6 commits into from
Feb 11, 2025

Conversation

kayagokalp
Copy link
Member

@kayagokalp kayagokalp commented Feb 7, 2025

Description

closes #6721.

There were couple of issues:

  1. Archive unpacking was getting the bytes from text field of response to the IPFS gateway which was adding additional stuff that messes with unpacking routine.
  2. Folder name was invalid so forc was unable to find the packages in the ipfs cache.

Both issues are addressed and this unblocks forc to fetch ipfs fetched packages once again. Next up on our package registry push is to enable alternative sources and set the precedence for them.

Also added some tests around extracting logic, which inserts the contents under a directory named after CID as this is how forc is looking for these sources in the first place (related to point 2 above)

@kayagokalp kayagokalp added forc-pkg Everything related to the `forc-pkg` crate. forc-registry Everything to do with forc-registry; IPFS sourcing, package registery labels Feb 7, 2025
@kayagokalp kayagokalp self-assigned this Feb 7, 2025
@kayagokalp kayagokalp marked this pull request as ready for review February 7, 2025 23:17
@kayagokalp kayagokalp requested review from a team as code owners February 7, 2025 23:17
@kayagokalp kayagokalp added the bug Something isn't working label Feb 7, 2025
Copy link

codspeed-hq bot commented Feb 7, 2025

CodSpeed Performance Report

Merging #6902 will not alter performance

Comparing kayagokalp/6721 (b948248) with master (1fdf630)

Summary

✅ 22 untouched benchmarks

JoshuaBatty
JoshuaBatty previously approved these changes Feb 8, 2025
Copy link
Member

@JoshuaBatty JoshuaBatty left a comment

Choose a reason for hiding this comment

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

LGTM

@JoshuaBatty JoshuaBatty requested a review from a team February 8, 2025 23:33
zees-dev
zees-dev previously approved these changes Feb 9, 2025
Copy link
Contributor

@zees-dev zees-dev left a comment

Choose a reason for hiding this comment

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

Raised a potential issue; don't want it to be a blocker though - hence approving.
LGTM 👍

Copy link
Contributor

@zees-dev zees-dev left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@kayagokalp kayagokalp enabled auto-merge (squash) February 11, 2025 00:19
@kayagokalp kayagokalp requested a review from a team February 11, 2025 00:19
@kayagokalp kayagokalp merged commit 0640dbe into master Feb 11, 2025
41 checks passed
@kayagokalp kayagokalp deleted the kayagokalp/6721 branch February 11, 2025 01:35
sdankel pushed a commit that referenced this pull request Feb 11, 2025
## Description
closes #6721.

There were couple of issues:

1. Archive unpacking was getting the bytes from `text` field of response
to the IPFS gateway which was adding additional stuff that messes with
unpacking routine.
2. Folder name was invalid so forc was unable to find the packages in
the `ipfs` cache.

Both issues are addressed and this unblocks forc to fetch ipfs fetched
packages once again. Next up on our package registry push is to enable
alternative sources and set the precedence for them.

Also added some tests around extracting logic, which inserts the
contents under a directory named after `CID` as this is how `forc` is
looking for these sources in the first place (related to point 2 above)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working forc-pkg Everything related to the `forc-pkg` crate. forc-registry Everything to do with forc-registry; IPFS sourcing, package registery
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Forc IPFS package fetching capabilities does not work with Pinata pinned packages
3 participants