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

Pulling in same WebJar with same version should be disallowed and handled as conflict #141

Open
mkurz opened this issue Jan 28, 2024 · 2 comments

Comments

@mkurz
Copy link

mkurz commented Jan 28, 2024

I set up a sample repository to show step by step what I consider a problem.

  1. git clone [email protected]:mkurz/same-webjar-different-type.git
  2. Read the step by step guide I set up:
    1. Starting with https://github.com/mkurz/same-webjar-different-type/blob/same-webjar_different-versions/README.md
    2. Afterwards https://github.com/mkurz/same-webjar-different-type/blob/same-webjar_same-version/README.md
(Copy pasting the READMEs here in case I ever delete that reproducer repo, click to expand)
We include three webjars, they have the same name, but different groupid and **different versions**:
 libraryDependencies ++= Seq( 
   "org.webjars.npm" % "prelude-ls" % "1.2.1", 
   "org.webjars.bower" % "prelude-ls" % "1.1.2", 
   "org.webjars" % "prelude-ls" % "1.1.1", 
 ) 

If you run:

$ sbt "clean; assets"
$ tree -L 1 ./target/web/web-modules/main/webjars/lib/prelude-ls
./target/web/web-modules/main/webjars/lib/prelude-ls
├── 1.1.1
├── 1.1.2
└── 1.2.1

4 directories, 0 files

You can see for each version a subfolder gets created.

This wasn't the case before webjars-locator-core#28 was merged.
Before that pull request, no subfolders got created, but all three webjars were merged directly into the prelude-ls folder.
So IMHO creating subfolders is the better approach now.

If you comment out two of the three webjars, so that only one webjars remains, then also no subfolders will be created, but the content of the one webjar will be put directly into the prelude-ls folder. This behaviour also seems reasonable, given the history of how it was done before.

But what if the exact same version is used for the above three webjars?
Lets find out in the same-webjar_same-version branch, take a look at its README!

In this branch we include three webjars, they have the same name and the same versions (but different groupid):

libraryDependencies ++= Seq(
  "org.webjars.npm" % "prelude-ls" % "1.1.1",
  "org.webjars.bower" % "prelude-ls" % "1.1.1",
  "org.webjars" % "prelude-ls" % "1.1.1",
)

Again, run:

$ git checkout same-webjar_same-version
$ sbt "clean; assets"
$ tree -L 1 ./target/web/web-modules/main/webjars/lib/prelude-ls
./target/web/web-modules/main/webjars/lib/prelude-ls
├── browser                   <-- folder from bower webjar
├── CHANGELOG.md              <-- file from bower webjar
├── .gitignore                <-- file from bower webjar
├── lib                       <-- folder from npm or bower webjar (who knows?)
├── LICENSE                   <-- file from npm or bower webjar (who knows?)
├── Makefile                  <-- file from bower webjar
├── package.json              <-- file from npm or bower webjar (who knows?)
├── package.json.ls           <-- file from bower webjar
├── prelude-browser.js        <-- file from classic webjar
├── prelude-browser-min.js    <-- file from classic webjar
├── preroll.ls                <-- file from bower webjar
├── README.md                 <-- file from npm or bower webjar (who knows?)
├── src                       <-- folder from bower webjar
├── test                      <-- folder from bower webjar
└── .travis.yml               <-- file from bower webjar

5 directories, 11 files

Now no subfolders get created (of course, it would be the same subfolder because they all have the same version).
But now the files of all three webjars are merged directly into the prelude-ls folder (see notes above).

IMHO this is not a good idea, because who knows which webjar overrides which? Which file(s) get(s) overriden?
I think such a conflict should fail hard with an exception like

Version conflict: WebJar "prelude-ls" with version "1.1.1" is present multiple times but with different groupids.

The thing is if I write an application or a library and include for example

"org.webjars" % "prelude-ls" % "1.1.1"`

I expect that this dependency (or better: the files from this WebJar) is immutable if it is present. Now maybe the files don't change for a while, but maybe I add another dependency to my project which pulls in prelude-ls 1.1.1, but the npm one... Now maybe that would now override the files and suddenly things can get weird...
This is why I think such a case should be disallowed.

@jamesward What do you think? Shouldn't the described case raise an exception since it's a version conflict? Specially that the contents of the files is not really immutable I think this is very very bad behaviour...

Side note on creating subfolders

Like mentioned above, subfolders were not always created in the past, but a relatively new behaviour, at least from Play's perspective, because in Play 2.8 we use an older webjars-locator version which did not create subfolders (like demonstrated, WebJar's contents got merged into one folder), whereas Play 2.9+ now uses a newer webjars-locator version which now creates subfolders.

What I experienced now is that using require(foolib) failed, because in Play 2.8, even when there was a "conflict", foolib would be directly available, even if the folder contents would be unpredictable, because WebJars could have been merged together. But now in Play 2.9+, because a project did actually pull in multiple webjars with the same name, subfolders for each version get created, so require(foolib) suddenly failed. To fix that I had to write require(foolib/1.2.3) (1.2.3 being the version). However... how would I know, if I write an sbt web plugin, if there is another sbt web library pulled in by a project which pulls in the same webjar, causing subfolders to be created? Like I can never be sure if I have to write require(foolib/1.2.3) or require(foolib), I never know for sure where the WebJar gets extracted too...
To solve this we need to come up with a fallback solution, which first looks for the WebJar in the subfolder and then, if that sub folder does not exist, in the parent folder. This way it is guaranteed require(...) doesn't fail - and actually also, if you do the lookup in that order, that you definitely are accessing the exact version which got defined in the sbt libraryDependencies (because if the version subfolder does not exists, it means there is only one webjar with that name pulled in and extracted in the parent folder, the one you defined in your project).
I implemented such a solution with the help of the node-require-fallback package (which comes with a very simple implemention), see here:

@jamesward What do you think about all of that?

@mkurz mkurz changed the title Pulling in same WebJar with same version should be disallowed and handled as conflict Pulling in same WebJar with _same_ version should be disallowed and handled as conflict Jan 28, 2024
@mkurz mkurz changed the title Pulling in same WebJar with _same_ version should be disallowed and handled as conflict Pulling in same WebJar with_same version should be disallowed and handled as conflict Jan 28, 2024
@mkurz mkurz changed the title Pulling in same WebJar with_same version should be disallowed and handled as conflict Pulling in same WebJar with same version should be disallowed and handled as conflict Jan 28, 2024
@mkurz
Copy link
Author

mkurz commented Jan 29, 2024

I had another interesting case, which I was not able to workaround with the require fallback approach:

libraryDependencies ++= Seq(
  "org.webjars.npm" % "mocha" % "3.0.0-2",
  "org.webjars" % "mocha" % "1.17.1",
)

Now both of those dependencies also depend on org.webjars[.npm]:debug:

Because they both depend a different version of debug webjar (one time the classic webjar, once the npm webjar, and also different version numbers), that means also for the debug webjar subfolders are created.
However, mocha does require('debug') here, which is hardcoded in the library, which mean this code fails (because debug now lives in a subfolder) and I can not work around that.
The only thing I can do to exclude one of the debug package in build.sbt, like

("org.webjars.npm" % "mocha" % "3.0.0-2")
    .exclude("org.webjars.npm", "debug")

@jamesward
Copy link
Member

Trying to wrap my head around this and remember some historical context... It seems like the extractor should preserve the webjars type and the version with subdirs, but is that viable? Thanks for bring this up and working on ways to tackle it.

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

2 participants