-
Notifications
You must be signed in to change notification settings - Fork 289
The Nix pipelines #1573
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
base: main
Are you sure you want to change the base?
The Nix pipelines #1573
Conversation
> add nix tools to devShell (rust-required stuff provided by default) > add build dependencies > add build profiles
By the way, as I am newbie to rust ecosystem, I suppose I did some bad configuration in toml configs. If you think so, let me know. |
.cargo/audit.toml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't work in the sandbox
What happens in the sandbox? Does it fail to send HTTP requests to crates.io?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did not checked (this is crane generated defaults) but I suppose yes it fails
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Makes sense if this file was generated by some tool.
But I don't think we'd want to disable audits for yanked crates just because a dev tool doesn't work well with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting up TOML formatting sounds good to me, but TBH I think this can go in a separate PR. That way we can get TOML formatting merged in faster while we still discuss flake.
flake.lock
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll have to research flake later, but could you discuss the pros of tracking the lock file in the repository?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same pros of tracking cargo.lock in repository - to be sure the shell will be SAME, and to be sure it is going to behave same way anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose the question is: do we need that amount of consistency? Or can we trust that the requirements and constraints in flake.nix
are most likely good enough?
Continuing with the comparison to Cargo.lock
, there are many times where you don't commit that lock file, because you don't need to use exact versions. This is common when authoring libraries, for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not including it would null most of nix's benefits. I have yet to find a single nix flake without its respective lockfile attached.
Just my 2cts here. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, that makes sense. But I believe having to regularly update the lock file was part of the reason that the initial attempt was reverted (#1549 (comment)).
I'm guessing that these benefits are being able to create a reproducible environment? IMO that's not always the highest priority -- our devcontainer config isn't that strict, for example (actually I think it's a bit too strict right now). Sometimes ease of setup, with the assumption that a well-functioning environment will be built, is enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's why I added workflow updating flake.lock
regularly (exactly, once in a week).
Although now I realized I didn't check it yet and don't really know how do I do that 😅.
I'm doing some research right now.
Also, (I didn't checked yet, but) I believe nix would refuse to work with flake.lock in .gitignore
I would also recommend using nix-direnv
|
I think I should recreate flake from scratch. Now I worked much more with crane and I can say it can be better. Much better |
I believe we should include clippy, deny and audit checks in our CI. Also it is possible to build CI with nix (this should speed up checks because of dependency caching) |
Please take a look at taplo.toml, I suppose you would not like some features enabled |
taplo.toml
Outdated
reorder_inline_tables = true | ||
reorder_keys = true | ||
# | ||
crlf = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely not CRLF for formatting.
We do try to alphabetize, so that makes sense. Other than that, I'll leave preferences up to @o2sh to decide.
I'll probably cherry-pick the taplo configuration, since it's not dependent on any sort of Nix configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Btw @o2sh this is formatted Cargo.toml
[workspace.package]
authors = [ "o2sh <ossama-hjaji@live.fr>" ]
edition = "2021"
license = "MIT"
repository = "https://github.com/o2sh/onefetch"
version = "2.23.1"
[workspace]
members = [ "ascii", "image", "manifest" ]
[workspace.dependencies]
anyhow = "1.0"
clap = { version = "4.5.26", features = [ "derive" ] }
image = { version = "0.25.5", default-features = false, features = [
"color_quant",
"jpeg",
"png",
"webp",
] }
owo-colors = "4.1.0"
strum = { version = "0.26.3", features = [ "derive" ] }
[package]
authors.workspace = true
categories = [ "command-line-utilities" ]
description = "Command-line Git information tool"
edition.workspace = true
exclude = [ "docs/vercel/*" ]
homepage = "https://onefetch.dev"
keywords = [ "cli", "git", "terminal" ]
license.workspace = true
name = "onefetch"
repository.workspace = true
rust-version = "1.81.0"
version.workspace = true
[dependencies]
anyhow.workspace = true
askalono = "0.5.0"
byte-unit = "5.1.6"
clap.workspace = true
clap_complete = "4.5.42"
crossbeam-channel = "0.5.14"
#
gix = { version = "0.70.0", default-features = false, features = [
"blob-diff",
"index",
"mailmap",
"max-performance-safe",
"status",
] }
#
gix-features = { version = "0.40.0", features = [ "zlib-ng" ] }
globset = "0.4.15"
human-panic = "2.0.2"
image.workspace = true
num-format = "0.4.4"
onefetch-ascii = { path = "ascii", version = "2.19.0" }
onefetch-image = { path = "image", version = "2.19.0" }
onefetch-manifest = { path = "manifest", version = "2.19.0" }
owo-colors.workspace = true
regex = "1.11.1"
serde = "1.0"
serde_json = "1.0"
serde_yaml = "0.9.34"
# TODO With the new value parsers, we're really close to being able to eliminate
# the strum dependency
strum.workspace = true
time = { version = "0.3.37", features = [ "formatting" ] }
time-humanize = { version = "0.1.3", features = [ "time" ] }
tokei = "13.0.0-alpha.8"
typetag = "0.2"
[dev-dependencies]
criterion = "0.5.1"
gix-testtools = "0.15.0"
insta = { version = "1.42.1", features = [ "json", "redactions" ] }
rstest = "0.24.0"
[[bench]]
harness = false
name = "repo"
[build-dependencies]
lazy_static = "1"
regex = "1"
serde_json = "1"
serde_yaml = "0.9"
tera = { version = "1", default-features = false }
[target.'cfg(windows)'.build-dependencies]
winres = "0.1"
[target.'cfg(windows)'.dependencies]
enable-ansi-support = "0.2.1"
[features]
fail-on-deprecated = [ ]
@Sk7Str1p3 Try to separate formatting from behavioral changes (features, fixes) in your commits. I.e. formatting should in a different commit from other tasks. The reason is that formatting commits can be added to |
Oh yes, I accidentally pushed formatted files, sorry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it could be interesting to consider treefmt-nix since it goes well with flakes.
P.D: Gotta love being able to comfortably build this project from latest git :p
It goes with flakes really well, I'm really tired of manually formatting Also I think we should bring here flake-parts |
@spenserblack will it be okay to have pull requests with same name like 'flake.lock: update'? I (almost) succeeded with workflow but I couldn't came up with idea how should we distinguish them. Do you have any ideas on that? |
OH I MANAGED TO ADD DATE THERE YAY |
@o2sh @spenserblack, PR is ready for merge, but... Main part is finished, but we can use all of this nix features in our CI. main advantage is that we can reuse built deps and let do it for everyone (who uses nix obviously), so this will leed to much faster, really blazing fast builds and checks. I can try implement all of these. So, what's your opinion on that? |
This pull request supersedes #1549 with much better approaches.
Features:
This can also be used in CI for much more powerful checks