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

feat(usePantry): Add support for multiple dists in distributable. #196

Closed
wants to merge 22 commits into from
Closed

Conversation

rustdevbtw
Copy link
Contributor

This PR makes distributable key take multiple (and conditional) dists.
For example:

distributable:
    - url: https://downloads.sourceforge.net/project/cscope/cscope/v{{version.raw}}/cscope-{{version.raw}}.tar.gz
      strip-components: 1
    - url: https://downloads.sourceforge.net/project/cscope/cscope/v{{version.raw}}/cscope-{{version.raw}}.tar.bz2
      strip-components: 1

Only the topmost available (200 OK) dist will be used.
This PR also adds support for version conditions (RegExp-based) and temporary rewrites. Such as:

distributable:
    - url: https://github.com/opentofu/opentofu/archive/refs/tags/v{{version.raw}}.tar.gz
      strip-components: 1
      if: ^(\d+)\.(\d+)\.(\d+)\.(\d+)$ # If the version matches this regex
      rewrite:
          # For example, 1.6.0.3 becomes 1.6.0-alpha3
          match: ^(\d+)\.(\d+)\.(\d+)\.(\d+)$
          with: $1.$2.$3-alpha$4
    - url: https://github.com/opentofu/opentofu/archive/refs/tags/v{{version.raw}}.tar.gz # For future, when it has stable releases (so manually changing it wouldn't be needed)
      strip-components: 1

@rustdevbtw
Copy link
Contributor Author

Seems like zlib.net and catb.org/wumpus's archive URLs are not working (could be either some version issue, or something, they both are 404)

@rustdevbtw
Copy link
Contributor Author

Others seems to pass the main step (getDistributable). So, this PR is now full backwards compatible (hopefully).

@rustdevbtw
Copy link
Contributor Author

Seems like for both zlib.net and catb.org/wumpus, if the patch is 0 (1.3 or 1.9 for example), it is the tags and releases wouldn't have that as well. So, maybe changing {{version.raw}} to {{version.marketing}} might help (at least for Wumpus, since it has no release with non-zero number in patch).

@rustdevbtw
Copy link
Contributor Author

However, with this PR (at least), it would be possible to simply do this:

distributable:
  # For normal cases
  - url: https://zlib.net/zlib-{{version.raw}}.tar.gz
    strip-components: 1
  # For minor releases (such as v1.3)
  - url: https://zlib.net/zlib-{{version.marketing}}.tar.gz
    strip-components: 1

Whatever works would be used.

@jhheider
Copy link
Contributor

Ok, the problem here is that pkg.constraint.single() isn't preserving version.raw. So your 404 checker is killing extract.ts and query.ts. this is the fix.

diff --git a/libexec/extract.ts b/libexec/extract.ts
index c2d7924..06e95d8 100755
--- a/libexec/extract.ts
+++ b/libexec/extract.ts
@@ -19,8 +19,7 @@ const { flags: { outputDir, pkg: pkgname }, unknown } = parseFlags(Deno.args, {
 
 const pantry = usePantry()
 
-let pkg: Package | PackageRequirement = utils.pkg.parse(pkgname)
-pkg = { project: pkg.project, version: pkg.constraint.single() ?? panic() }
+const pkg: Package | PackageRequirement = await usePantry().resolve(utils.pkg.parse(pkgname)) ?? panic
 
 const dstdir = Path.cwd().join(outputDir)
 
diff --git a/libexec/query.ts b/libexec/query.ts
index bc4bc42..2e52c7d 100755
--- a/libexec/query.ts
+++ b/libexec/query.ts
@@ -40,7 +40,7 @@ if (versions) {
   Deno.exit(0)
 }
 
-const version = pkg.constraint.single() ?? panic()
+const {version} = await usePantry().resolve(pkg) ?? panic()
 pkg = {project: pkg.project, version }
 
 if (src) {

@mxcl i'll open a ticket for libpkgx, but does that change frighten or disturb you in any way?

@jhheider
Copy link
Contributor

recommend simplifying the code thusly:

diff --git a/lib/usePantry.ts b/lib/usePantry.ts
index a832885..981d4ff 100644
--- a/lib/usePantry.ts
+++ b/lib/usePantry.ts
@@ -93,11 +93,7 @@ const getDistributable = async (pkg: Package) => {
   const moustaches = useMoustaches();
 
   const yml = await hooks.usePantry().project(pkg).yaml();
-  let final_url = "";
-  let dists = yml.distributable;
-  if (!dists) dists = [];
-  let stripComponents: number | undefined;
-  if (!isArray(dists)) dists = [dists];
+  const dists = isArray(yml.distributable) ? yml.distributable : [yml.distributable]
   for (const dist of dists) {
     //FIXME: Add check for Git dists as well
     if (dist.git) {
@@ -106,65 +102,46 @@ const getDistributable = async (pkg: Package) => {
       );
       return getGitDistribution({ pkg, ...dist });
     }
+
     if (dist.url?.startsWith("git")) {
       return getGitDistribution({ pkg, ...dist });
     }
+
     let urlstr = getRawDistributableURL(dist);
-    let raw_v = "";
-    let matched = true;
     if (!urlstr) continue;
-    let tmp_stripComponents: number | undefined;
+
+    const v = pkg.version
+    const stripComponents = flatmap(dist["strip-components"], coerceNumber)
+
     if (isPlainObject(dist)) {
-      tmp_stripComponents = flatmap(dist["strip-components"], coerceNumber);
       if (dist.rewrite?.match) {
-        raw_v = pkg.version.raw.replace(
+        v.raw = v.raw.replace(
           new RegExp(dist.rewrite["match"], "gi"),
           dist.rewrite["with"],
         );
       }
+
       if (dist?.if) {
-        matched = new RegExp(dist.if).test(pkg.version.raw);
+        const matched = new RegExp(dist.if).test(pkg.version.raw);
+        if (!matched) continue
       }
     }
-    let v: SemVer;
-    if (raw_v) {
-      v = {
-        raw: raw_v,
-        major: pkg.version.major,
-        minor: pkg.version.minor,
-        patch: pkg.version.patch,
-        components: pkg.version.components,
-        prerelease: pkg.version.prerelease,
-        build: pkg.version.build,
-        eq: pkg.version.eq,
-        neq: pkg.version.neq,
-        gt: pkg.version.gt,
-        gte: pkg.version.gte,
-        lt: pkg.version.lt,
-        lte: pkg.version.lte,
-        compare: pkg.version.compare,
-      };
-    } else {
-      v = pkg.version;
-    }
+
     urlstr = moustaches.apply(urlstr, [
       ...moustaches.tokenize.version(v),
       ...moustaches.tokenize.host(),
     ]);
-    if (!matched) continue;
+
     const rsp = await fetch(urlstr, { method: "HEAD" });
+
     if (rsp.status == 200) {
-      final_url = urlstr;
-      stripComponents = tmp_stripComponents;
-      break;
+      const url = new URL(urlstr);
+      return { url, ref: undefined, stripComponents, type: "url" }
     } else {
       console.warn(`brewkit: Could not fetch ${urlstr} [${rsp.status}]`)
     }
   }
-  if (!final_url) return;
-  const url = new URL(final_url);
-
-  return { url, ref: undefined, stripComponents, type: "url" }
+  return;
 }
 
 // deno-lint-ignore no-explicit-any

@jhheider
Copy link
Contributor

i believe that gets rid of all those mutable interior variables.

@rustdevbtw
Copy link
Contributor Author

i believe that gets rid of all those mutable interior variables.

I'm on mobile rn, can you apply these changes please?

@jhheider
Copy link
Contributor

i wasn't able to push to your repo, but let me try the web editor.

@jhheider
Copy link
Contributor

nope. can't edit the file. i can paste the whole thing if you want it

usePantry.ts
import { isNumber, isPlainObject, isString, isArray, PlainObject } from "is-what"
import { Package, PackageRequirement, SemVer, semver, utils, hooks } from "libpkgx"
import { getScript } from "./usePantry.getScript.ts"
import getVersions from "./usePantry.getVersions.ts"

const { flatmap, validate } = utils
const { useMoustaches } = hooks

export interface Interpreter {
  project: string
  args: string[]
}

export default function() {
  const foo = hooks.usePantry()
  return {
    getVersions,
    getDistributable,
    getScript,
    getPlatforms,
    resolve,
    getDeps,
    filepath,
    ...foo
  }
}

const getDeps = async (pkg: Package | PackageRequirement) => {
  const { parse_pkgs_node, project } = hooks.usePantry()
  const yml = await project(pkg).yaml()
  const runtime = parse_pkgs_node(yml.dependencies)
  const build = parse_pkgs_node(yml.build?.dependencies)
  const test = parse_pkgs_node(yml.test?.dependencies)
  return { runtime, build, test }
}

async function resolve(spec: Package | PackageRequirement): Promise<Package> {
  const constraint = "constraint" in spec ? spec.constraint : new semver.Range(`=${spec.version}`)
  const versions = await getVersions(spec)
  const version = constraint.max(versions)
  if (!version) {
    console.error({versions})
    throw new Error(`not-found: version: ${utils.pkg.str(spec)}`)
  }
  console.debug({selected: version})
  return { project: spec.project, version };
}

const getPlatforms = async (pkg: Package | PackageRequirement) => {
  let { platforms } = await hooks.usePantry().project(pkg).yaml()
  if (!platforms) return ["linux/x86-64", "linux/aarch64", "darwin/x86-64", "darwin/aarch64"]
  if (isString(platforms)) platforms = [platforms]
  if (!isArray(platforms)) throw new Error(`invalid platform node: ${platforms}`)
  const rv = []
  for (const platform of platforms) {
    if (platform.match(/^(linux|darwin)\/(aarch64|x86-64)$/)) rv.push(platform)
    else if (platform.match(/^(linux|darwin)$/)) rv.push(`${platform}/x86-64`, `${platform}/aarch64`)
    else throw new Error(`invalid platform: ${platform}`)
  }
  return rv
}

const getRawDistributableURL = (dist: PlainObject) => {
  if (isPlainObject(dist)) {
    return validate.str(dist.url);
  } else if (isString(dist)) {
    return dist;
  } else if (dist === null || dist === undefined) {
    return;
  } else {
    throw new Error(`invalid distributable node: ${dist}`);
  }
};

const getGitDistribution = ({ pkg, url: urlstr, ref }: { pkg: Package, url: string, ref: string }) => {
  if (!ref) {
    throw new Error("distributable.ref is required because we mirror source tarballs even when cloning from git")
  }

  const url = new URL(urlstr.replace(/^git\+http/, 'http'))

  const moustaches = useMoustaches()

  ref = moustaches.apply(ref, [
    ...moustaches.tokenize.version(pkg.version),
    ...moustaches.tokenize.host()
  ])

  return { url, ref, stripComponents: 0, type: 'git' }
}

const getDistributable = async (pkg: Package) => {
  const moustaches = useMoustaches();

  const yml = await hooks.usePantry().project(pkg).yaml();
  const dists = isArray(yml.distributable) ? yml.distributable : [yml.distributable]
  for (const dist of dists) {
    //FIXME: Add check for Git dists as well
    if (dist.git) {
      console.warn(
        "brewkit: using distributable.git instead of distributable.url is deprecated",
      );
      return getGitDistribution({ pkg, ...dist });
    }

    if (dist.url?.startsWith("git")) {
      return getGitDistribution({ pkg, ...dist });
    }

    let urlstr = getRawDistributableURL(dist);
    if (!urlstr) continue;

    const v = pkg.version
    const stripComponents = flatmap(dist["strip-components"], coerceNumber)

    if (isPlainObject(dist)) {
      if (dist.rewrite?.match) {
        v.raw = v.raw.replace(
          new RegExp(dist.rewrite["match"], "gi"),
          dist.rewrite["with"],
        );
      }

      if (dist?.if) {
        const matched = new RegExp(dist.if).test(pkg.version.raw);
        if (!matched) continue
      }
    }

    urlstr = moustaches.apply(urlstr, [
      ...moustaches.tokenize.version(v),
      ...moustaches.tokenize.host(),
    ]);

    const rsp = await fetch(urlstr, { method: "HEAD" });

    if (rsp.status == 200) {
      const url = new URL(urlstr);
      return { url, ref: undefined, stripComponents, type: "url" }
    } else {
      console.warn(`brewkit: Could not fetch ${urlstr} [${rsp.status}]`)
    }
  }
  return;
}

// deno-lint-ignore no-explicit-any
function coerceNumber(input: any) {
  if (isNumber(input)) return input
}

//FIXME inefficient, should be in libpkgx as part of .project()
async function filepath(project: string) {
  for await (const pkg of hooks.usePantry().ls()) {
    if (project == pkg.project) return pkg.path
  }
  throw new Error(`package.yml not found: ${project}`)
}

@rustdevbtw
Copy link
Contributor Author

nope. can't edit the file. i can paste the whole thing if you want it

usePantry.ts

Done

@jhheider
Copy link
Contributor

looks like it needs an if (!dists) return between:

  const yml = await hooks.usePantry().project(pkg).yaml();
  const dists = isArray(yml.distributable) ? yml.distributable : [yml.distributable]

putting it after will never catch.

@jhheider
Copy link
Contributor

your test of dists has to come a line up. it'll never be falsy after its set.

@rustdevbtw
Copy link
Contributor Author

your test of dists has to come a line up. it'll never be falsy after its set.

I've changed it to a check within the loop

@mxcl
Copy link
Member

mxcl commented Oct 31, 2023

K, so these are mirrors. Why do we need to add this complexity?

@jhheider
Copy link
Contributor

mirrors isn't the only reason. initial idea was fallback when we found that "edit a release to make it appear in the github api" issue. but for things like linux-kernel-headers or cscope, where releases have to be scanned for from a number of URLs, it would make that possible.

@jhheider
Copy link
Contributor

jhheider commented Oct 31, 2023

or pkgxdev/pantry@a82c31e where they added a v to their version tag for that release.

@rustdevbtw
Copy link
Contributor Author

Seems like some network issue in the last workflow run for vim

@mxcl
Copy link
Member

mxcl commented Nov 17, 2023

k that rationale makes sense, sorry for the delay here. I've been deep in CEO duties for weeks. We need another me.

libexec/query.ts Outdated
@@ -40,7 +40,7 @@ if (versions) {
Deno.exit(0)
}

const version = pkg.constraint.single() ?? panic()
const {version} = await usePantry().resolve(pkg) ?? panic()
Copy link
Member

Choose a reason for hiding this comment

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

yeah this changes the behavior and is thus not acceptable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, this change was suggested by @jhheider as a hack to preserve version.raw for single constraints. (Some like zlib were breaking because version.raw included trailing .0, which they don't expect.

Copy link
Contributor Author

@rustdevbtw rustdevbtw Nov 17, 2023

Choose a reason for hiding this comment

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

We can also probably do this:

const pkg_version = pkg.constraint.single() ?? panic()
const pantry_ver = await usePantry().resolve(pkg) ?? panic()
const version = (pkg_version.raw == pantry_ver.version.raw) ? pkg_version : pantry_ver.version

It'll have the same behaviour for cases where the .raw matches (for normal cases). For others, it'll change that to fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mxcl Any suggestions?

@rustdevbtw
Copy link
Contributor Author

@mxcl @jhheider Since Brewkit v1 has been released (which is a good thing), I think this PR is no longer valid, and I think I should redo it.

@rustdevbtw rustdevbtw closed this Dec 13, 2023
@mxcl
Copy link
Member

mxcl commented Dec 13, 2023

I apologize for wasting your time. I was going to reimplement it for you. Up to you.

rustdevbtw added a commit to rustdevbtw/brewkit that referenced this pull request Dec 13, 2023
@rustdevbtw
Copy link
Contributor Author

I apologize for wasting your time. I was going to reimplement it for you. Up to you.

No need to apologize. I really enjoying contributing to pkgx, it's a really cool project.

mxcl added a commit that referenced this pull request Dec 13, 2023
…214)

* redo of #196
* typo
* Add a fixture for multiple distributables
* fix

---------

Co-authored-by: Max Howell <mxcl@me.com>
mxcl added a commit that referenced this pull request Dec 13, 2023
…214)

* redo of #196
* typo
* Add a fixture for multiple distributables
* fix

---------

Co-authored-by: Max Howell <mxcl@me.com>
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.

3 participants