Skip to content

Pedantry #98

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Pedantry #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6b2bf81
Changed lowrisc_pkgs -> lowriscPkgs
HU90m Apr 1, 2025
b7bb917
Changed llvm-cheriot identifier.
HU90m Apr 1, 2025
ecea895
Renamed python_ot to python-ot
HU90m Apr 1, 2025
16521b4
Renamed bazel_ot to bazelisk
HU90m Apr 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions dev/opentitan.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
pkgs,
ncurses5-fhs,
Copy link
Contributor

@hcallahan-lowrisc hcallahan-lowrisc Apr 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Re. ecea895, the commit message doesn't seem to match the changes again? This appears to just be a snake -> kebab change for the nix binding of opentitan's python environment.
I think the spirit of the message is correct though, we should probably give the naming another thought at some time. Ideally 'python' is distinct from a 'pythonEnv', but also should the pname contain the project that particular env is intended for? I'm undecided. I almost think that it would be better if we could namespace as e.g. packages.${system}.opentitan.pythonEnv, but flakes doesn't allow that output schema.

ncurses6-fhs,
bazel_ot,
python_ot,
bazelisk,
python-ot,
verilator_ot,
verible_ot,
edaTools ? [],
Expand All @@ -18,7 +18,7 @@
...
}: let
# These dependencies are required for building user DPI C/C++ code, and cosimulation models.
edaExtraDeps = with pkgs; [elfutils openssl python_ot];
edaExtraDeps = with pkgs; [elfutils openssl python-ot];

gcc-patched = wrapCCWith {
cc = gcc-unwrapped;
Expand Down Expand Up @@ -46,13 +46,13 @@ in
targetPkgs = _:
with pkgs;
[
bazel_ot
bazelisk
verilator_ot
verible_ot

# Python dependencies
uv
python_ot
python-ot

# For serde-annotate which can be built with just cargo
rustup
Expand Down
10 changes: 5 additions & 5 deletions flake.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Re. the naming for let bindings, is there a standard somewhere that camel case is preferred? Or maybe just that camel > kebab > snake? I see all kinds of stuff. Maybe we could setup a lint if there is a standard somewhere

Copy link
Collaborator

Choose a reason for hiding this comment

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

https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md says use camel case for (non-package-name) variable names.

];
};
lowrisc_pkgs = import ./pkgs {inherit pkgs inputs;};
lowriscPkgs = import ./pkgs {inherit pkgs inputs;};
in {
checks = {
license = pkgs.stdenv.mkDerivation {
Expand All @@ -73,23 +73,23 @@
'';
};
};
packages = flake-utils.lib.filterPackages system lowrisc_pkgs;
packages = flake-utils.lib.filterPackages system lowriscPkgs;
devShells = {
opentitan = pkgs.callPackage ./dev/opentitan.nix {
inherit (lowrisc_pkgs) ncurses5-fhs ncurses6-fhs bazel_ot verilator_ot python_ot verible_ot;
inherit (lowriscPkgs) ncurses5-fhs ncurses6-fhs bazelisk verilator_ot python-ot verible_ot;
};
cheriot = pkgs.mkShell {
Copy link
Contributor

Choose a reason for hiding this comment

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

Re. b7bb917, the commit message doesn't seem to match the changes? The change is just snake -> kebab

name = "cheriot";
packages =
(with lowrisc_pkgs; [llvm_cheriot xmake])
(with lowriscPkgs; [llvm-cheriot xmake])
++ (with pkgs; [
gnumake
magic-enum
srecord
]);
};
caliptra = pkgs.callPackage ./dev/caliptra.nix {
inherit (lowrisc_pkgs) verilator_caliptra riscv64-gcc_caliptra;
inherit (lowriscPkgs) verilator_caliptra riscv64-gcc_caliptra;
};
};
formatter = pkgs.alejandra;
Expand Down
0 pkgs/bazel_ot/default.nix → pkgs/bazelisk/default.nix
View file
Open in desktop
File renamed without changes.
6 changes: 3 additions & 3 deletions pkgs/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@

# OpenTitan packages
verilator_ot = import ./verilator {inherit pkgs;};
python_ot = pkgs.callPackage ./python_ot {inherit inputs;};
bazel_ot = pkgs.callPackage ./bazel_ot {};
python-ot = pkgs.callPackage ./python-ot {inherit inputs;};
bazelisk = pkgs.callPackage ./bazelisk {};
verible_ot = pkgs.callPackage ./verible.nix {};

# CherIoT packages
spike-ibex-cosim = pkgs.callPackage ./spike.nix {};
llvm_cheriot = pkgs.callPackage ./llvm_cheriot.nix {};
llvm-cheriot = pkgs.callPackage ./llvm-cheriot.nix {};
xmake = pkgs.callPackage ./xmake {};
cheriot-sim = pkgs.callPackage ./cheriot-sim.nix {};
cheriot-audit = pkgs.callPackage ./cheriot-audit.nix {};
Expand Down
0 pkgs/llvm_cheriot.nix → pkgs/llvm-cheriot.nix
View file
Open in desktop
File renamed without changes.
0 pkgs/python_ot/default.nix → pkgs/python-ot/default.nix
View file
Open in desktop
File renamed without changes.
0 pkgs/python_ot/pyproject.toml → pkgs/python-ot/pyproject.toml
View file
Open in desktop
File renamed without changes.
0 pkgs/python_ot/uv.lock → pkgs/python-ot/uv.lock
View file
Open in desktop
File renamed without changes.