Support short version like tmux/ijg

[uesyn]

It was failed to upload tmux, because tmux is released with shorter versions like 3.3a.
In this PR, support uploading tmux short version.

refs: pkgxdev/pantry#1665 https://github.com/teaxyz/pantry/actions/runs/4823537544/jobs/8592369144#logs

[mxcl]

😬 that regex. Not your fault. But scary for me. @jhheider thoughts?

[jhheider]

There's also software like libjpeg whose most recent version is 9e. So, if we solve it for \d+\.\d+[[:alpha:]]?, we might as well get \d+[[:alpha:]]? as well. I think that change looks small, but as you note: regexes can have huge effect cascades.

I'd be happier if we had a unit/integration test that ensured reasonable results out of:

1
1.2
1.2.3
1.2.3.4
1a
1.2a
1.2.3a
1.2.3.4a

[uesyn]

@mxcl @jhheider
I re-wrote this PR to support versions which doesn't include dot like 9e or 9, and add tests for them.
Considering the architecture == x86-64, It could not be expressed in one regex in my power.
I wasn't sure where to place the unit tests, so I made the tests/unit direcotry and placed the tests in the directory.
Please comment on any details that need to be corrected👍

[jhheider]

I will definitely need to test how it's handled throughout teaxyz/cli. I'm surprised semver.ts didn't need tweaks, but maybe https://github.com/teaxyz/cli/blob/31c2fc0bbddc41cbd63d9ee14593efe3f9b09301/src/utils/semver.ts#LL32C18-L32C18 handles all of the weirdness.

If you want to combine those regexen, btw, I think this is the right one:

const cacheRegex = `^(.*)-(\\d+(\\.\\d+)*[:alpha:]?)(\\+(darwin|linux)\\+(aarch64|x86-64))?\\.$`

Of note: I believe the version schema we're accepting is \d+(\.\d+)*[:alpha:]?, and files in the cache may have other extentions (tgz, tar.bz2, tbz2, zip, at a minimum).

[uesyn]

If you want to combine those regexen, btw, I think this is the right one:

In case of arch matches x86-64, the regex doesn't work well, because x86-64's - is matched due to longuest match 😢
Therefore, I had no choice but to split the regex...

regex test

import { assertEquals } from "https://deno.land/std@0.185.0/testing/asserts.ts";

Deno.test(function regexTest() {
  const cacheRegex = `^(.*)-(\\d+(\\.\\d+)*[:alpha:]?)(\\+(darwin|linux)\\+(aarch64|x86-64))?\\.tar.[gx]z$`
  const path = "project∕foo-1.2.3+linux+x86-64.tar.gz";
  const match = path.match(cacheRegex)!
  const {pkg, version, platform} = {pkg: match[0], version: match[1], platform: match[2]}
  assertEquals(pkg, "project∕foo")
  assertEquals(version, "1.2.3")
  assertEquals(platform, "+linux+x86-64")
});

running 1 test from ./main_test.ts
regexTest ... FAILED (9ms)

 ERRORS

regexTest => ./main_test.ts:3:6
error: AssertionError: Values are not equal.


    [Diff] Actual / Expected


-   project∕foo-1.2.3+linux+x86-64.tar.gz
+   project∕foo


  throw new AssertionError(message);
        ^
    at assertEquals (https://deno.land/std@0.185.0/testing/asserts.ts:189:9)
    at regexTest (file:///home/uesyn/src/tmp/ts/main_test.ts:8:3)

 FAILURES

regexTest => ./main_test.ts:3:6

FAILED | 0 passed | 1 failed (35ms)

error: Test failed

Of note: I believe the version schema we're accepting is \d+(.\d+)*[:alpha:]?, and files in the cache may have other extentions (tgz, tar.bz2, tbz2, zip, at a minimum).

For now, these extention is not used because it is not acceptable by the current regex, but should I also support these extention too in this PR?

This PR only makes bottles having shorter version uploaded correctly(e.g. tmux), and I believe this PR doesn't bring breaking changes to tea cli.
To be this PR accepted, should I make the PR to check whether shorter versions are accepted by tea cli?
I'm sorry that I'm not good at english speaker, so I don't think I understood everything you said. If my understanding is wrong, let me know🙇

[jhheider]

I think you're getting it right. You might be able to solve the x86-64 problem by making the first term ^([^\+]*) on the assumption that we'll never have a package name with a + in it.

I would say we probably need to be permissive w.r.t. the file extensions, so we don't introduce future maintenance issues. Adding github.com/tmux/tmux to the test matrix in ci.yml is probably the best first step, and I'd add ijg.org as well, so we can quickly test it if/when we determine that moving to parsable versioning for 9e works.

[jhheider]

It looks really good. It seems to work with ijg.org if you change its versions key to:

versions:
  url: https://ijg.org/files/
  match: /jpegsrc\.v\d+[a-z]?\.tar\.gz/
  strip:
    - /jpegsrc\.v/
    - /\.tar\.gz/

as well (built 9a and 9e locally). I'm ready to sign-off and see if/what it breaks, since this is brewkit and cli already seems to accept it.

[jhheider]

Willing to roll the dice!

[jhheider]

Hm. Interestingly, ijg.org seems to already work with version 9e if https://github.com/teaxyz/pantry/actions/runs/4876431031 is to be believed... I wonder what's special about tmux...

[jhheider]

ah, right, it breaks down on upload. Fortunately, this will be fixed before the next release of ijg.org, certainly.

[jhheider]

Interesting; it looks like it has fixed x86-64, but not aarch64. we might need to take another crack at the regex.

https://github.com/teaxyz/pantry/actions/runs/4876470207/jobs/8720571042

[jhheider]

Though it worked just fine for tmux.

So, the specific case of $pkg-\d+[a-z]+$platform+aarch64 isn't handled correctly.

[mxcl]

I recommend doing \+(\w+) and matching the build identifier manually. Not everything has to be in the regex

[uesyn]

@jhheider @mxcl

Thank you for your review!

oh... I'm sorry, It's my mistake. I think regex [:alpha:] mathes only :, l, p, h, or a.

In my tests, only suffix a is tested, so tests were passed.
We should use [a-z] or \w instead of [:alpha:].
I can make a PR to fix this problem soon.

[uesyn]

I made the PR to fix it: #106