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: refactor auto update #124

Merged
merged 4 commits into from
Nov 6, 2024
Merged

feat: refactor auto update #124

merged 4 commits into from
Nov 6, 2024

Conversation

oluceps
Copy link
Contributor

@oluceps oluceps commented Nov 5, 2024

Summary

Add package layout change. This change is backward compatible.

Proposed features

After this change, we could get rid of syncing between different branches, reduce CI time cost. Users will be able to switch between versions without changing flake inputs.

Other branches are expected to be identical to the main for a while (with warning) and then deprecate.

On main branch:

2a3,5
>     ├───dae-experiment: package 'dae-v0.7.0' - 'A Linux high-performa…'
>     ├───dae-release: package 'dae-v0.8.0' - 'A Linux high-performance…'
>     ├───dae-unstable: package 'dae-unstable-2024-11-03.d3ab0b2' - 'A …'

dae is now an alias of dae-release.

Move metadata.json

To top-level and contains all revision info about dae.
Other package info could be added future.

{
  "dae": {
    "release": {
      "version": "v0.8.0",
      "rev": "v0.8.0",
      "hash": "sha256-Vdh5acE5i/bJ8VXOm+9OqZQbxvqv4TS/t0DDfBs/K5g=",
      "vendorHash": "sha256-0Q+1cXUu4EH4qkGlK6BIpv4dCdtSKjb1RbLi5Xfjcew="
    },
    "unstable": {
//...
    },
    "experiment": {
 //...
    }
  }
}

Auto-update script

try with:

./main.nu sync
# also with extra args `unstable` `release` +

In nixpkgs, buildGoModule is during refactoring. The overriding in this repo is expected to be more elegant.

Action items

as far as maintenance concerned, add relevant maintenance notes to highlight the following:

if the revision you provided doesn't match any of ["release" "unstable"], it will: check the --rev args, read its value, run nix-prefetch-git and get relative info, adding a new record to metadata.json, then update the vendorHash

  • Completed

Copy link
Contributor

@sumire88 sumire88 left a comment

Choose a reason for hiding this comment

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

Thanks for raising this. I really like the idea of unifying versions in the main branch. However, before we proceed, let's get down to a discussion of various challenges we need to deal with.

Comment on lines +3 to +8
"release": {
"version": "v0.8.0",
"rev": "v0.8.0",
"hash": "sha256-Vdh5acE5i/bJ8VXOm+9OqZQbxvqv4TS/t0DDfBs/K5g=",
"vendorHash": "sha256-0Q+1cXUu4EH4qkGlK6BIpv4dCdtSKjb1RbLi5Xfjcew="
},
Copy link
Contributor

Choose a reason for hiding this comment

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

The way how we distribute new upstream releases is by creating annotated tag (e.g. ref/tags/dae-v0.9.0rc1). Any thoughts on how to deal with prerelease? We need a way to archive history releases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The way how we distribute new upstream releases is by creating annotated tag (e.g. ref/tags/dae-v0.9.0rc1). Any thoughts on how to deal with prerelease? We need a way to archive history releases.

Based on #124 (comment) I'll add a new function for the script, to create the new specified version in metadata.json. No change needed in flake.nix.

metadata.json Outdated
Comment on lines 15 to 20
"experiment": {
"version": "v0.7.0",
"rev": "v0.7.0",
"hash": "sha256-9iwrwQGpryGyEUVB2reodIxuEQHkXPA4P5IYKj18elI=",
"vendorHash": "sha256-AtYLxR7Fw3+IOSeuPXlq4vMsnS+7MMaFANZDg0yvCl8="
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The purpose of having the experiment branch is to test new features or bug fixes which are proposed in upstream PR branches. Any thoughts on how we can update the metadata in main branch based on needs? One way to achieve so is creating a dedicated pipeline (workflow) to fetch the latest commit in an upstream feature branch and pass the sha or ref as input and dynamically update the metadata.json file.

Copy link
Contributor Author

@oluceps oluceps Nov 6, 2024

Choose a reason for hiding this comment

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

Any thoughts on how we can update the metadata in main branch based on needs?

Here we can add a new cmd arg for specifying revision hash for experiment.
The interface will looks like:

./main.nu sync dae prereleaz --rev ec7cf06de4afa173a46307d5d3095f67e97a1ffd

Copy link
Contributor

@sumire88 sumire88 Nov 6, 2024

Choose a reason for hiding this comment

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

Any thoughts on how we can update the metadata in main branch based on needs?

Here we can add a new cmd arg for specifying revision hash for experiment. The interface will looks like:

./main.nu sync dae prereleaz --rev ec7cf06de4afa173a46307d5d3095f67e97a1ffd

Let's make it intuitive:

# ref (latest)
./main.nu sync dae prerelease

Ideally, we should not be dealing with sha manually (passing any rev/sha). If we were to pin a specific prerelease then we may use:

./main.nu sync dae prerelease --rev 'refs/tags/v0.9.0rc1'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's make it intuitive:

The string prereleaz could be any words, it will sync according the provided rev and update the metadata record with this key (whatever it was). See #124 (comment)

@sumire88
Copy link
Contributor

sumire88 commented Nov 5, 2024

As far as I understand, we would like to build a collection of binaries and distribute them to users with a single input source.

2a3,5
>     ├───dae-experiment: package 'dae-v0.7.0' - 'A Linux high-performa…'
>     ├───dae-release: package 'dae-v0.8.0' - 'A Linux high-performance…'
>     ├───dae-unstable: package 'dae-unstable-2024-11-03.d3ab0b2' - 'A …'

This is an excellent idea, as I recently benefit from a package, namely scx.full from the group of nyx package maintainers. They have the following module options:

_:

{
  environment.systemPackages = with pkgs; [ scx ];

  chaotic.scx = {
    enable = true;
    package = pkgs.scx;
    # one of "scx_bpfland", "scx_central", "scx_flatcg", "scx_lavd", "scx_layered", "scx_nest", "scx_pair", "scx_qmap", "scx_rlfifo", "scx_rustland", "scx_rusty", "scx_simple", "scx_userland"
    scheduler = "scx_bpfland";
  };
}

Ref: chaotic-cx/nyx#910

This may serve as a good reference for us. I would like to hear from you about your thoughts on this!

@oluceps
Copy link
Contributor Author

oluceps commented Nov 6, 2024

This is an excellent idea, as I recently benefit from a package, namely scx.full from the group of nyx package maintainers. They have the following module options:

The scx package layout in nyx repo be like:

nix-repl> packages.x86_64-linux.scx_git.
packages.x86_64-linux.scx_git.bpfland   packages.x86_64-linux.scx_git.rlfifo
packages.x86_64-linux.scx_git.cscheds   packages.x86_64-linux.scx_git.rustland
packages.x86_64-linux.scx_git.full      packages.x86_64-linux.scx_git.rusty
packages.x86_64-linux.scx_git.lavd
packages.x86_64-linux.scx_git.layered

If we change to, like dae.unstable dae.experiment, the package dae will become a meta package which causes backward incompatibility, this requires users to change their configuration to make it work. And idk if it fits well with garnix.

@oluceps
Copy link
Contributor Author

oluceps commented Nov 6, 2024

Updated.

> ./main.nu sync dae pre --rev 'refs/tags/v0.9.0rc1'
2024-11-06T09:40:01.320|INF|adding new version
2024-11-06T09:40:06.972|INF|save file for calc vendorHash
2024-11-06T09:41:32.963|INF|save final file
+        ├───dae-pre: package 'dae-unstable-2024-11-03.d3ab0b2' - 'A Linux…'

@oluceps oluceps requested a review from sumire88 November 6, 2024 09:44
@sumire88
Copy link
Contributor

sumire88 commented Nov 6, 2024

This is an excellent idea, as I recently benefit from a package, namely scx.full from the group of nyx package maintainers. They have the following module options:

The scx package layout in nyx repo be like:

nix-repl> packages.x86_64-linux.scx_git.
packages.x86_64-linux.scx_git.bpfland   packages.x86_64-linux.scx_git.rlfifo
packages.x86_64-linux.scx_git.cscheds   packages.x86_64-linux.scx_git.rustland
packages.x86_64-linux.scx_git.full      packages.x86_64-linux.scx_git.rusty
packages.x86_64-linux.scx_git.lavd
packages.x86_64-linux.scx_git.layered

If we change to, like dae.unstable dae.experiment, the package dae will become a meta package which causes backward incompatibility, this requires users to change their configuration to make it work. And idk if it fits well with garnix.

Yes, this is a big concern. That's why I originally came up with the idea of maintaining three branches, passing the version control to the user through controlling flake input. The new solution doesn't seem to solve this backward-compatibility issue for now.

@sumire88
Copy link
Contributor

sumire88 commented Nov 6, 2024

Updated.

> ./main.nu sync dae pre --rev 'refs/tags/v0.9.0rc1'
2024-11-06T09:40:01.320|INF|adding new version
2024-11-06T09:40:06.972|INF|save file for calc vendorHash
2024-11-06T09:41:32.963|INF|save final file
+        ├───dae-pre: package 'dae-unstable-2024-11-03.d3ab0b2' - 'A Linux…'

Let's make it more intuitive:

./main.nu sync dae prerelease --rev 'refs/tags/v0.9.0rc1'

./main.nu sync dae unstable --rev 'refs/heads/main'

./main.nu sync dae experiment --rev '<sha>'

@oluceps
Copy link
Contributor Author

oluceps commented Nov 6, 2024

Updated.

> ./main.nu sync dae pre --rev 'refs/tags/v0.9.0rc1'
2024-11-06T09:40:01.320|INF|adding new version
2024-11-06T09:40:06.972|INF|save file for calc vendorHash
2024-11-06T09:41:32.963|INF|save final file
+        ├───dae-pre: package 'dae-unstable-2024-11-03.d3ab0b2' - 'A Linux…'

Let's make it more intuitive:

./main.nu sync dae prerelease --rev 'refs/tags/v0.9.0rc1'

./main.nu sync dae unstable --rev 'refs/heads/main'

./main.nu sync dae experiment --rev '<sha>'

The --rev args could pass in with:

  • revision hash
  • refs/heads/
  • refs/tags/v0.0.0

As far as I tried.

Workflow for updating release and unstable:

./main.nu sync dae release unstable # or leave the last 2 args empty

workflow for updating single version:

./main.nu sync dae release # or unstable

workflow for adding a new version:

./main.nu sync dae sth-new --rev 'rev_hash' or refs/heads/<branch> or refs/tags/v0.0.0 

after this will produce a new package called dae-sth-new

@oluceps
Copy link
Contributor Author

oluceps commented Nov 6, 2024

You could try with the current branch, I've removed the experiment revision from metadata.json since it's outdated.

./main.nu sync dae experiment --rev 'refs/tags/v0.9.0rc1'

@sumire88
Copy link
Contributor

sumire88 commented Nov 6, 2024

You could try with the current branch, I've removed the experiment revision from metadata.json since it's outdated.

./main.nu sync dae experiment --rev 'refs/tags/v0.9.0rc1'

I am still confused. The experiment entry has been removed from metadata.json, so how would it work then?

@oluceps
Copy link
Contributor Author

oluceps commented Nov 6, 2024

You could try with the current branch, I've removed the experiment revision from metadata.json since it's outdated.

./main.nu sync dae experiment --rev 'refs/tags/v0.9.0rc1'

I am still confused. The experiment entry has been removed from metadata.json, so how would it work then?

The overall execution path is:

if the revision you provided doesn't match any of ["release" "unstable"], it will:
check the --rev args, read its value, run nix-prefetch-git and get relative info, adding a new record to metadata.json, then update the vendorHash

@sumire88
Copy link
Contributor

sumire88 commented Nov 6, 2024

You could try with the current branch, I've removed the experiment revision from metadata.json since it's outdated.

./main.nu sync dae experiment --rev 'refs/tags/v0.9.0rc1'

I am still confused. The experiment entry has been removed from metadata.json, so how would it work then?

The overall execution path is:

if the revision you provided doesn't match any of ["release" "unstable"], it will: check the --rev args, read its value, run nix-prefetch-git and get relative info, adding a new record to metadata.json, then update the vendorHash

Ok, got it now. We need to document it in our maintenance notes. I've mentioned it in the PR description.

@oluceps oluceps force-pushed the dev branch 3 times, most recently from eb910ad to 401c2c8 Compare November 6, 2024 15:21
@oluceps
Copy link
Contributor Author

oluceps commented Nov 6, 2024

Ok, got it now. We need to document it in our maintenance notes. I've mentioned it in the PR description.

Done.

Other branches are expected to be identical to the main for a while (with warning) and then deprecate.

@sumire88 Any plan about deprecating other branches? If it's on schedule I'll update some deprecation warning to these branches after this pr merged, for notifying users while they evaling.

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
@sumire88
Copy link
Contributor

sumire88 commented Nov 6, 2024

Ok, got it now. We need to document it in our maintenance notes. I've mentioned it in the PR description.

Done.

Other branches are expected to be identical to the main for a while (with warning) and then deprecate.

@sumire88 Any plan about deprecating other branches? If it's on schedule I'll update some deprecation warning to these branches after this pr merged, for notifying users while they evaling.

Before we deprecate other branches, we need to also refactor the current CI. I propose we open up an issue to discuss this topic further.

Copy link
Contributor

@sumire88 sumire88 left a comment

Choose a reason for hiding this comment

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

LGTM. Let's proceed with the changes.

Co-authored-by:    Sumire (菫) <151038614+sumire88@users.noreply.github.com>
@oluceps oluceps merged commit a50ac81 into main Nov 6, 2024
12 checks passed
@oluceps oluceps deleted the dev branch November 6, 2024 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants