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

Relative path to source file in sourcemap is broken sometimes #3135

Closed
glmars opened this issue Sep 8, 2017 · 5 comments
Closed

Relative path to source file in sourcemap is broken sometimes #3135

glmars opened this issue Sep 8, 2017 · 5 comments
Assignees
Labels
bug Confirmed bug. Needs to be fixed.
Milestone

Comments

@glmars
Copy link

glmars commented Sep 8, 2017

I found stable reproduction (see example project https://github.com/glmars/scalajs-wrongsourcemap)

Steps to reproduce

  1. run sbt clean fastOptJS first time
  2. check generated source map file ./target/scala-2.11/scalajs-wrongsourcemap-fastopt.js.map, there is such path to source file:
    "../src/main/scala/ru/lmars/bundler/Application.scala"
    
    this is wrong path!
  3. run sbt clean fastOptJS second time
  4. path to source file will be different:
    "../../src/main/scala/ru/lmars/bundler/Application.scala"
    
    and this is right path!
  5. delete directory ./target/scala-2.11
  6. run sbt clean fastOptJS
  7. source file path in .map file is broken again!

I ran into this bug many times when my file manager holds some directories inside target and sbt clean couldn't delete them (but successfully deletes ./target/scala-2.11).

@sjrd
Copy link
Member

sjrd commented Sep 8, 2017

Thanks for the report. I can indeed reproduce this. The reliable way to reproduce it is to start sbt (or reload it) at a time when the target/scala-2.11 directory does not exist. This happens because of this line:

Some((artifactPath in key).value.getParentFile.toURI())

It is incorrect because getParentFile will return a path without a trailing /. Then, .toURI fixes it with a trailing / but only if the file already exists and is a directory. Since this line of code is evaluated at the time sbt is loaded, it will do the wrong thing if target/scala-2.11 does not exist at that time.

The fix is to ensure that there is a trailing / in the URI in all cases.

@sjrd sjrd self-assigned this Sep 8, 2017
@sjrd sjrd added the bug Confirmed bug. Needs to be fixed. label Sep 8, 2017
@sjrd sjrd added this to the v0.6.21 milestone Sep 8, 2017
@sjrd
Copy link
Member

sjrd commented Sep 9, 2017

In fact, there is an even better way: don't use getParentFile at all. Relativizing URIs is perfectly capable of relativizing against a file rather than a directory. This is even more correct, since the browser will then resolve the relative URI based on the file URI, not its parent directory.

@sjrd
Copy link
Member

sjrd commented Sep 9, 2017

@glmars Workaround that you can use right now:

scalaJSLinkerConfig := {
  val fastOptJSURI = (artifactPath in (Compile, fastOptJS)).value.toURI
  scalaJSLinkerConfig.value.withRelativizeSourceMapBase(Some(fastOptJSURI))
}

In fact, you'll have to do that in 1.x anyway, because relativeSourceMaps does not exist there, replaced by scalaJSLinkerConfig.

sjrd added a commit to sjrd/scala-js that referenced this issue Sep 9, 2017
… .js file.

Instead of the parent directory of the .js file. This solves the
issue that the parent directory did not have a trailing `/` in some
cases.
gzm0 added a commit that referenced this issue Sep 9, 2017
[no-master] Fix #3135: Relativize source map URIs against the .js file.
@sjrd
Copy link
Member

sjrd commented Sep 9, 2017

Fixed in 4d8f39a.

@sjrd sjrd closed this as completed Sep 9, 2017
@glmars
Copy link
Author

glmars commented Sep 10, 2017

@sjrd, thank you for your attention even to such a minor (for someone) bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug. Needs to be fixed.
Projects
None yet
Development

No branches or pull requests

2 participants