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

Chisel rename fails on case-insensitive filesystems. #219

Closed
ucbjrl opened this issue Jun 9, 2016 · 6 comments
Closed

Chisel rename fails on case-insensitive filesystems. #219

ucbjrl opened this issue Jun 9, 2016 · 6 comments

Comments

@ucbjrl
Copy link
Contributor

ucbjrl commented Jun 9, 2016

Unfortunately, we can't provide both a "Chisel" package and a "chisel" package on platforms with case-insensitive file systems (i.e., Mac OSX):

[warn] .../chisel3/src/main/scala/chisel/compatibility.scala:6: Class Chisel.package$ differs only in case from chisel.package$. Such classes will overwrite one another on case-insensitive filesystems.
[warn] package object Chisel {
[warn]                ^

Running tests in chisel3 result in:

[info] Compiling 36 Scala sources to /tmp/jrltmp/sbt/Chisel3/scala-2.11/test-classes...
[error] .../chisel3/src/test/scala/chiselTests/Assert.scala:7: object testers is not a member of package chisel
[error] import chisel.testers.BasicTester
[error]               ^
[error] .../chisel3/src/test/scala/chiselTests/Assert.scala:8: object util is not a member of package chisel
[error] import chisel.util._
[error]               ^
[error] .../chisel3/src/test/scala/chiselTests/Assert.scala:10: not found: type BasicTester
[error] class FailingAssertTester() extends BasicTester {
[error]                                     ^
[error] .../chisel3/src/test/scala/chiselTests/Assert.scala:11: not found: value Bool
[error]   assert(Bool(false))
[error]          ^
[error] .../chisel3/src/test/scala/chiselTests/Assert.scala:13: not found: value Counter
[error]   val (_, done) = Counter(!reset, 4)
[error]                   ^
[error] .../chisel3/src/test/scala/chiselTests/Assert.scala:13: not found: value reset

We can either:

  • require case-sensitive filesystems for Chisel development
  • eliminate the "Chisel" package and compatibility layer
  • "temporarily" rename the lowercase package name to "chisel3"
  • skip the rename altogether

Although OSX suggests the root volume should be case-insensitive, additional volumes may be configured as case-sensitive. A case-sensitive filesystem is recommended for Android development (and they give instructions for setting up a suitable environment on OSX - https://source.android.com/source/initializing.html )

@aswaterman
Copy link
Member

Many software projects fall victim to this, including even the GNU C Library. We mount case-sensitive file systems to get around that issue. It is annoying and would hinder uptake to some extent.

Can we handle this another way by putting the two packages in different directory subtrees?

Alternatively, using chisel3 as the package name doesn't sound like a bad idea.

@ducky64
Copy link
Contributor

ducky64 commented Jun 10, 2016

I tried putting the Chisel stuff in a different package in the caseFix branch, but that doesn't have seemed to work on my Windows machine. Anyone on a case-insensitive Mac want to try it?

@ucbjrl
Copy link
Contributor Author

ucbjrl commented Jun 10, 2016

It appears to compile on an OSX case-insensitive FS without warnings about case sensitivity, but almost all tests fail with:

[info] Exception encountered when attempting to run a suite with class name: chiselTests.DirectionSpec *** ABORTED ***
[info]   java.lang.NoClassDefFoundError: chisel/package$
[info]   at chisel.Driver$$anonfun$elaborate$1.apply(Driver.scala:110)
[info]   at chisel.Driver$$anonfun$elaborate$1.apply(Driver.scala:110)
[info]   at chisel.internal.Builder$$anonfun$build$1.apply(Builder.scala:116)
[info]   at chisel.internal.Builder$$anonfun$build$1.apply(Builder.scala:114)
[info]   at scala.util.DynamicVariable.withValue(DynamicVariable.scala:58)
[info]   at chisel.internal.Builder$.build(Builder.scala:114)
[info]   at chisel.Driver$.elaborate(Driver.scala:110)
[info]   at chiselTests.ChiselRunners$class.elaborate(ChiselSpec.scala:23)
[info]   at chiselTests.ChiselPropSpec.elaborate(ChiselSpec.scala:31)
[info]   at chiselTests.DirectionSpec$$anonfun$1.apply$mcV$sp(Direction.scala:30)
[info]   ...

On Jun 9, 2016, at 6:17 PM, Richard Lin notifications@github.com wrote:

I tried putting the Chisel stuff in a different package in the caseFix branch, but that doesn't have seemed to work on my Windows machine. Anyone on a case-insensitive Mac want to try it?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@ducky64
Copy link
Contributor

ducky64 commented Jun 10, 2016

Yeah, got the same problem. I'm guessing that regardless of the project structure we give it, the classes get flattened at runtime and still collide, it just gets pushed past where the compiler could warn you about it

@chick
Copy link
Contributor

chick commented Jun 11, 2016

Let me place my vote for having chisel having only the lower case form and conversion to chisel3 require fixing up the package names in your project. There is a growing list of things that have to be done to move to chisel3, this one more does not seem that onerous. I also think we are using up a lot of time trying to solve nearly intractable problems like this

@ducky64
Copy link
Contributor

ducky64 commented Jun 11, 2016

Having users move to lowercase chisel was always part of the plan, but that's not due to happen until the "release". This is mainly "how do we support both until everyone is ready to make the huge jump" (compatibility mode).

Another question is how much work needs to go into compatibility mode vs. how much the user needs to put in (for example, perhaps requiring case-sensitive filesystems during the transition period is acceptable)?

@ducky64 ducky64 closed this as completed Nov 21, 2016
mwachs5 pushed a commit that referenced this issue Dec 29, 2022
Bumps [chisel3](https://github.com/freechipsproject/chisel3) from `74a727f` to `c146fa0`.
- [Release notes](https://github.com/freechipsproject/chisel3/releases)
- [Commits](74a727f...c146fa0)

---
updated-dependencies:
- dependency-name: chisel3
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
jackkoenig added a commit that referenced this issue Feb 28, 2023
Fixes #219

* Adds AsyncResetType (similar to ClockType)
* Registers with reset signal of type AsyncResetType are async reset
  registers
* Registers with async reset can only be reset to literal values
* Add initialization logic for async reset registers
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

No branches or pull requests

4 participants