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

Change default layers to use lowercase directories #4505

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

seldridge
Copy link
Member

Change directories for default layers to use lowercase output directories. While this is independently likely a better choice, it is a somewhat pragmatic, as these directory names match the legacy names of firtool's extract-test-code feature (which was a custom SiFive pass for the SFC before being open-sourced in firtool). This will help us land an extract-test-code replacement without having to disturb build flows. Secondarily, and independently, lower case directories are likely better.

There are two alternative changes which were rejected:

  1. The layers could be renamed to be lowercase. Generally, it's better to have objects and classes be uppercase.

  2. The automatic directory names could be mangled to be lowercase as opposed to the exact layer names.

Release Notes

Change directories for default layers to be lowercase.

Change directories for default layers to use lowercase output directories.
While this is independently likely a better choice, it is a somewhat
pragmatic, as these directory names match the legacy names of `firtool`'s
extract-test-code feature (which was a custom SiFive pass for the SFC
before being open-sourced in `firtool`).  This will help us land an
extract-test-code replacement without having to disturb build flows.
Secondarily, and independently, lower case directories are likely better.

There are two alternative changes which were rejected:

1. The layers could be renamed to be lowercase.  Generally, it's better to
   have objects and classes be uppercase.

2. The automatic directory names could be mangled to be lowercase as
   opposed to the exact layer names.

Signed-off-by: Schuyler Eldridge <[email protected]>
@seldridge seldridge requested review from jackkoenig and rwy7 November 13, 2024 22:44
@seldridge seldridge added API Modification Backend Code Generation Affects backend code generation, will be included in release notes and removed API Modification labels Nov 13, 2024
Copy link
Contributor

@jackkoenig jackkoenig left a comment

Choose a reason for hiding this comment

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

LGTM. I think this is better default behavior

@seldridge seldridge enabled auto-merge (squash) November 13, 2024 23:37
@seldridge seldridge merged commit 0bb1bce into main Nov 13, 2024
14 checks passed
@seldridge seldridge deleted the dev/seldridge/change-default-layer-directories branch November 13, 2024 23:56
Emin017 added a commit to Emin017/chisel-nix that referenced this pull request Feb 1, 2025
Chisel will generate lowercase directoreise by default now, see:
chipsalliance/chisel#4505

Signed-off-by: Qiming Chu <[email protected]>
Emin017 added a commit to Emin017/chisel-nix that referenced this pull request Feb 1, 2025
Chisel will generate lowercase directoreise by default now, see:
chipsalliance/chisel#4505.

Signed-off-by: Qiming Chu <[email protected]>
Avimitin pushed a commit to chipsalliance/chisel-nix that referenced this pull request Feb 2, 2025
Chisel will generate lowercase directoreise by default now, see:
chipsalliance/chisel#4505.

Signed-off-by: Qiming Chu <[email protected]>
Avimitin pushed a commit to chipsalliance/chisel-nix that referenced this pull request Feb 8, 2025
Chisel will generate lowercase directoreise by default now, see:
chipsalliance/chisel#4505.

Signed-off-by: Qiming Chu <[email protected]>
unlsycn added a commit to chipsalliance/chisel-nix that referenced this pull request Feb 9, 2025
* [build] bump to mill 0.12.5 and refactor build scripts

Signed-off-by: unlsycn <[email protected]>

* [nix] remove projectDependencies scripts

Signed-off-by: unlsycn <[email protected]>

* [nix] fix add-determinsim and mill-deps building on darwin

* bump add-determinism to unstable-2024-11-12 and add patches for darwin
* fix mill-deps building on darwin (The caches dir on darwin is
`$TMPDIR/Library/Caches/Coursier`)

* [nix] remove fix-darwin.patch and use fetchpatch

fetchpatch from
https://github.com/Emin017/add-determinism/commits/fix-darwin/

Signed-off-by: Qiming Chu <[email protected]>

* [nix] Use determinism to handle hash mismatch in chisel publishLocal

Signed-off-by: Qiming Chu <[email protected]>

* [nix] differentiate hash for darwin in gcd.nix

Signed-off-by: Qiming Chu <[email protected]>

* [nix] change layers folders to use lowercase directories

Chisel will generate lowercase directoreise by default now, see:
chipsalliance/chisel#4505.

Signed-off-by: Qiming Chu <[email protected]>

* [ci] fix mill env

* [nix] remove local packed add-determinism and use upstream version

Signed-off-by: Qiming Chu <[email protected]>

* [nix] use mill flow overlay

Signed-off-by: unlsycn <[email protected]>

* [doc] update readme.md

Signed-off-by: unlsycn <[email protected]>

---------

Signed-off-by: unlsycn <[email protected]>
Signed-off-by: Qiming Chu <[email protected]>
Co-authored-by: Qiming Chu <[email protected]>
Co-authored-by: Avimitin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend Code Generation Affects backend code generation, will be included in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants