Skip to content

Fix runtime finding in v2 endpoint#476

Merged
HexF merged 7 commits into
engineer-man:nix-packagesfrom
Brikaa:fix-v2-endpoint
Jun 7, 2022
Merged

Fix runtime finding in v2 endpoint#476
HexF merged 7 commits into
engineer-man:nix-packagesfrom
Brikaa:fix-v2-endpoint

Conversation

@Brikaa
Copy link
Copy Markdown
Member

@Brikaa Brikaa commented May 2, 2022

No description provided.

Copy link
Copy Markdown
Collaborator

@HexF HexF left a comment

Choose a reason for hiding this comment

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

I think it would be nice to allow some SemVer-like patterns instead of just hard-coding *.
Obviously we can't use actual SemVer as Nix doesn't force versions into SemVer, but maybe we can try determine the versioning scheme (e.g. CalVer, SemVer, etc.) and allow matching on them?

@Brikaa
Copy link
Copy Markdown
Member Author

Brikaa commented May 17, 2022

@HexF how about we have a configurable and changeable "mainstream" version for each package. It could be stored in some file as a package to version map or something. Then "*" would just get this mainstream version. Other versions would be specified manually.

This will also help solve the problem of multiple runtimes of the same package

@HexF
Copy link
Copy Markdown
Collaborator

HexF commented May 27, 2022

@Brikaa Issue with that is we would have to pin nixpkgs to a version to ensure that all the version numbers are the same. But adding to that, what do we consider as a version from CalVer to SemVer for example?

@Brikaa
Copy link
Copy Markdown
Member Author

Brikaa commented May 27, 2022

What I mean is that we would not need to do any version comparisons or conversions. We just add a mainstream_versions object in the config that would include something like:

'python3': '3.10.4',
'bash': '5.1'

Choosing * as the version would bring this package version. Otherwise, it would see if runtimes has a version that matches the provided version string.

@HexF
Copy link
Copy Markdown
Collaborator

HexF commented May 27, 2022

Ok yeah, that could work - build that map into the nix flake.

I'm thinking instead of the exact version rather it would link back to the runtime, e.g.:

{
  pistonMainstreamVersions = {
    "python3" = runtimes.python;
  };
}

@Brikaa
Copy link
Copy Markdown
Member Author

Brikaa commented May 30, 2022

I have got another idea. If we could somehow use an ordered data structure in runtimes/default.nix, we would just need to move the mainstream runtime to the top and it would take priority over other runtimes when selecting the * version. This can also be useful when multiple runtimes share the same aliases; the runtime that is more at the top of default.nix would take priority.

@Brikaa
Copy link
Copy Markdown
Member Author

Brikaa commented May 31, 2022

Or we can have another file that includes the priority order of the runtimes

@HexF
Copy link
Copy Markdown
Collaborator

HexF commented Jun 2, 2022

Not entirely sure if the nix-to-json conversion allows reading the order of elements in a map.

I'm using the flake to export all the available runtimes to piston, so ideally I would want to bundle all the version priority stuff into the flake.
I think a priority list would be best compared to hard-coding in runtimes for specific aliases, getting that merged in here would be great.

@Brikaa Brikaa force-pushed the fix-v2-endpoint branch from 896c25a to b50af13 Compare June 4, 2022 21:27
@Brikaa Brikaa requested a review from HexF June 4, 2022 21:30
@Brikaa
Copy link
Copy Markdown
Member Author

Brikaa commented Jun 4, 2022

Should we get rid of the v3 endpoint now?

@Brikaa
Copy link
Copy Markdown
Member Author

Brikaa commented Jun 5, 2022

This works by loading the runtimes mentioned in pistonMainstreamRuntimes in flake.nix first then loading the rest of the runtimes.

Choosing * as the version will pick the first runtime matching the request data (which will be the mainstream runtime if one is defined).

It also works with specific sets like bash-only and none.

Copy link
Copy Markdown
Collaborator

@HexF HexF left a comment

Choose a reason for hiding this comment

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

Looks good!

@HexF HexF merged commit c7bcc7f into engineer-man:nix-packages Jun 7, 2022
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.

2 participants