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

Correctly exclude base from Path.allSubpaths result #285

Merged
merged 1 commit into from
Jan 22, 2020
Merged

Correctly exclude base from Path.allSubpaths result #285

merged 1 commit into from
Jan 22, 2020

Conversation

eatkins
Copy link
Contributor

@eatkins eatkins commented Jan 21, 2020

It was reported in scala/scala#8525 that sbt
does not correctly exclude the base directory when calling
Path.allSubpaths. This ended up causing jar files to be created with an
empty entry. The problem was in an incorrect equality check because the
compiler did not flag for us that
PathFinder(base).globRecursive(filter).get() returned Seq[File] but
that we were filtering the results by comparing a File to a Path.

It was reported in scala/scala#8525 that sbt
does not correctly exclude the base directory when calling
Path.allSubpaths. This ended up causing jar files to be created with an
empty entry. The problem was in an incorrect equality check because the
compiler did not flag for us that
`PathFinder(base).globRecursive(filter).get()` returned `Seq[File]` but
that we were filtering the results by comparing a `File` to a `Path`.
PathFinder(base).globRecursive(filter).get().collect {
case f if f != path => f -> path.relativize(f.toPath).toString
case f if f != base => f -> base.toPath.relativize(f.toPath).toString
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if there could have been some sort of linting that could've caught this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I'm sort of surprised that I didn't get an unrelated type warning. From what I can tell, this seems baked into scala2: scala/scala3#1247. I wonder if there is a compiler plugin though that can catch this kind of thing.

Choose a reason for hiding this comment

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

It does feel like help should be nearby.

val f = new java.io.File("f"); val p = f.toPath; f match { case `p` => }

By coincidence, I just tweaked a similar expression albeit different file and path. I tend to stick to nio.file.Path now. There is a gotcha in that p.endsWith("f") is not String::endsWith.

@dwijnand dwijnand merged commit c369154 into sbt:develop Jan 22, 2020
@dwijnand
Copy link
Member

No backport candidate flag?

@eed3si9n
Copy link
Member

Created the label.

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.

4 participants