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

fetchipfs: support api/v0/get #136688

Closed
wants to merge 1 commit into from
Closed

fetchipfs: support api/v0/get #136688

wants to merge 1 commit into from

Conversation

milahu
Copy link
Contributor

@milahu milahu commented Sep 4, 2021

Motivation for this change

the old fetchipfs only works, when the object is an "IPFS tarchive"

when the object is a directory, the api method tar/cat fails:

curl -X POST "http://localhost:5001/api/v0/tar/cat?arg=bafybeiflkjt66aetfgcrgvv75izymd5kc47g6luepqmfq6zsf5w6ueth6y"
# {"Message":"not an IPFS tarchive","Code":0,"Type":"error"}

curl -s -w "%{http_code}" -o /dev/null -X POST "http://localhost:5001/api/v0/tar/cat?arg=bafybeiflkjt66aetfgcrgvv75izymd5kc47g6luepqmfq6zsf5w6ueth6y"
# 500

instead, we must use the api method get

curl -o output.tar -X POST "http://localhost:5001/api/v0/get?archive=true&compress=false&arg=bafybeiflkjt66aetfgcrgvv75izymd5kc47g6luepqmfq6zsf5w6ueth6y"

this patch will also

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

test

# fetchipfs-test.nix
# nix-build -E 'with import <nixpkgs> { }; callPackage ./fetchipfs-test.nix { }'
{ curl, stdenv }:
let
  fetchipfs = import ./fetchipfs { inherit curl stdenv; };
in
fetchipfs {
  # ipfs-desktop/assets/webui = 20 megabyte
  ipfs = "bafybeiflkjt66aetfgcrgvv75izymd5kc47g6luepqmfq6zsf5w6ueth6y";
  sha256 = "0p8w97j6rxnackm14p9npbra5a82irdb50i80qjc0pfpzjk781dm";
}

todo

  • test with other ipfs objects
  • test the retry loop
  • support fetching from public gateways = https://ipfs.io/api/v0/ etc
  • allow to add a human-readable name as suffix to store path
  • in the store path, always add the prefix ipfs- before the ipfs hash, so its easier to find ipfs objects in the store

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Sep 4, 2021
@Artturin
Copy link
Member

Artturin commented Sep 8, 2021

@knupfer

@milahu
Copy link
Contributor Author

milahu commented Sep 8, 2021

a small test script

#!/usr/bin/env bash

# test nixos fetchipfs
# requires ../fetchipfs from
# https://github.com/NixOS/nixpkgs/tree/0d35cc9d2b393f5e700f508ed859a2406884311b/pkgs/build-support/fetchipfs

# TODO use ipfs http api to add tarchive files
# https://docs.ipfs.io/reference/http/api/#api-v0-tar-add

set -e # exit on first error
set -o xtrace # print all commands

pidof ipfs || { echo start ipfs daemon; ipfs daemon & sleep 5; }

test() {
  local path="$1"
  out="$(ipfs add --recursive "$path" 2>/dev/null)"
  ipfs_hash=$(echo "$out" | tail -n1 | cut -d' ' -f2)
  sha256recursive=$(nix-hash --type sha256 "$path" | cut -d' ' -f1)

  cat >fetchipfs-test.nix <<EOF
{ curl, stdenv }:
let fetchipfs = import ../fetchipfs { inherit curl stdenv; }; in
fetchipfs { ipfs = "${ipfs_hash}"; sha256 = "${sha256recursive}"; }
EOF
  nix-build -E 'with import <nixpkgs> { }; callPackage ./fetchipfs-test.nix { }'
}

dd if=/dev/urandom bs=1024 count=1 status=none | base64 >random.txt
test random.txt
rm random.txt

mkdir random || true # allow to fail
dd if=/dev/urandom bs=1024 count=1 status=none | base64 >random/a.txt
dd if=/dev/urandom bs=1024 count=1 status=none | base64 >random/b.txt
test random
rm -rf random

echo pass

@Artturin
Copy link
Member

Artturin commented Sep 8, 2021

if you want to make tests then this will be useful #136022

@Artturin
Copy link
Member

The fetcher test pr has been merged

@stale
Copy link

stale bot commented Apr 16, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 16, 2022
@Luflosi
Copy link
Contributor

Luflosi commented Jul 4, 2022

What's the status of this PR? I'd like to get this merged if it doesn't break anything. Adding a test would be ideal.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2022
@milahu
Copy link
Contributor Author

milahu commented Jul 5, 2022

What's the status of this PR?

abandoned by OP

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 7, 2023
@Artturin
Copy link
Member

Artturin commented Jan 9, 2023

What's the status of this PR? I'd like to get this merged if it doesn't break anything. Adding a test would be ideal.

theres 0 uses of fetchipfs in nixpkgs so i don't want to merge this without tests
@Luflosi

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 9, 2023
@Luflosi
Copy link
Contributor

Luflosi commented Jan 11, 2023

I tried to do that (there are two commits) but gave up because I felt it was too difficult to preserve the hashes if I remember correctly. I think I also couldn't reproduce the hash of the source code from the hello package from when fetchipfs was first introduced, so maybe fetchipfs is not even reproducible.
The tar add / tar cat functions are now deprecated in Kubo.

There are many problems with the current state of things (even after applying this PR):
What if the IPFS API is not at localhost:5001 but at localhost:5002 or some-other-host.lan:5001? What if the file specified in the url input is already a tar archive? Then the archive is unpacked and repacked for no reason. The url input of fetchipfs only supports, well..., URLs. What if instead of fetching from a URL, the file should be downloaded from a git repository? Or from an FTP site? I don't like how fetchipfs basically reimplements fetchurl, but worse.

I think the whole concept of how things are fetched and unpacked in Nixpkgs needs to be reworked.
What if one wants to fetchzip but from IPFS? Do we need fetchzipipfs? That seems silly. What if one wants to download a tar archive from a git repo?
I think it would be better to split the downloading and unpacking derivations and to be able to somehow specify multiple alternative fetchers, in case one fails, for example because the file is no longer available. Then one could do things like download a file either from the original website or from an FTP site or from archive.org or from IPFS, based on and where the file is still available and maybe also based on user preference. Then the file could be decompressed and unpacked in a separate derivation. As long as the result is hashed after downloading or after unpacking, it should still work. Hashing after unpacking would require something like nondeterministic floating CA derivations I think. One could even chain multiple unpacking derivations if the file is like a tar archive of a zip or something.
I dislike how fetchgit downloads the entire source code every time. I think it should instead download a (shallow?) clone of the repo and keep it in a cache and fetch from that instead. The git commit or tag still determines the content, just as before. Changing the git commit would then just refresh the cache by downloading only the differences.

But I fear this idea is too crazy and complicated to ever be implemented. So now I am in a state where I dislike how ugly things are currently but don't really know how to improve them without too much effort and without breaking backwards compatibility.

@milahu
Copy link
Contributor Author

milahu commented Jan 11, 2023

somehow specify multiple alternative fetchers, in case one fails, for example because the file is no longer available. Then one could do things like download a file either from the original website or from an FTP site or from archive.org or from IPFS, based on and where the file is still available and maybe also based on user preference.

sounds like metalink
but yeah, nix fetchers were not designed to fetch from multiple protocols
but then, we can always use a custom fixed-output-derivation

What if the file specified in the url input is already a tar archive? Then the archive is unpacked and repacked for no reason.

then the first request succeeds with http 200.
only when the first request fails with http 500, we change the api endpoint

The tar add / tar cat functions are now deprecated in Kubo.

deprecated in favor of /api/v0/add and /api/v0/get, see ipfs/kubo#7951

What if the IPFS API is not at localhost:5001

use ipfs config Addresses.API
or better, add/get files with the ipfs command instead of curl
(i guess we use curl for better bootstrapping of nixpkgs, but then we still need ipfs daemon)

ipfs add -r some_folder/
ipfs get some_cid -o some_name

theres 0 uses of fetchipfs in nixpkgs

yepp. i needed this to build ipfs-desktop, where assets (images, etc) are stored in ipfs

@domenkozar
Copy link
Member

@domenkozar domenkozar closed this Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: fetch 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants