openSUSE / obs-service-node_modules

MIT License
7 stars 11 forks source link

Support multiple versions of the same package #33

Closed Lunarequest closed 1 year ago

Lunarequest commented 1 year ago

obs-service-node_modules does not support multiple versions of the same package, it appears multiple versions overwrite each other and as often javascript projects have multiple versions of the same package through peer deps. This causes build issues and in the worst case could cause silent miscompliation issues

AdamMajer commented 1 year ago

Can you provide an example?

What is assumed is that upstream appends version to the tarball which generally seems to happen. For example, some of the archives from cockpit package,

@babel-supports-color-5.5.0.tgz
balanced-match-1.0.2.tgz            
binary-extensions-2.2.0.tgz              
brace-expansion-1.1.11.tgz               
braces-3.0.2.tgz                            
builtins-5.0.1.tgz             
builtins-semver-7.5.4.tgz         
call-bind-1.0.2.tgz                       
callsites-3.1.0.tgz             
camelcase-5.3.1.tgz               
camelcase-keys-6.2.2.tgz          
chalk-4.1.2.tgz               
chokidar-3.5.3.tgz             
chrome-remote-interface-0.32.2.tgz
color-convert-2.0.1.tgz                
colord-2.9.3.tgz                
color-name-1.1.4.tgz         
commander-2.11.0.tgz                   
concat-map-0.0.1.tgz                    
core-util-is-1.0.3.tgz                
cosmiconfig-7.1.0.tgz               
cross-spawn-7.0.3.tgz         
cssesc-3.0.0.tgz                       
css-functions-list-3.2.0.tgz           
cssnano-preset-lite-2.1.3.tgz                                               
Nykseli commented 1 year ago

Hi!

I created a package on OBS to demonstrate the issue: https://build.opensuse.org/package/show/home:malikirri/cockpit-wicked-playground

If you run osc service manualrun you get this error:

ERROR:@testing-library/react/node_modules/aria-query: mismatch https://registry.npmjs.org/@types/aria-query/-/aria-query-4.2.2.tgz <> https://registry.npmjs.org/aria-query/-/aria-query-4.2.2.tgz, sha512:1e7629004d58ea447228cfd7904ba24508531ef933301bab4c79e914b60b0463c8ca564d3ecf632
5cb8e3985b13cb242484b66653d18f2b1c3a2428deeb4538a <> sha512:a3f1de97086e2a94e3fdfaec3ac6cd2cd82734654816c54ffd25b6052175e20565ee401f30e27affcc14014bc6d51d4d1caadf1bedac1d94eed326ae4f5a81ac                                                                                              

So it's trying to match SHA sums of two different packages that share the .tgz filename.

And if you try to build it (like you can see from the OBS logs), it fails to find the correct aria-query module:

npm ERR! notarget No matching version found for aria-query@^4.2.2.

Even though it's defined on the line 3706 in package-lock.json

AdamMajer commented 1 year ago

Thank you for the example. Looks like

      "resolved": "https://registry.npmjs.org/@types/aria-query/-/aria-query-4.2.2.tgz",
...
      "resolved": "https://registry.npmjs.org/aria-query/-/aria-query-4.2.2.tgz",

This should be prepended with scope like in the node_moduels, so @types/aria-query/-/aria-query-4.2.2.tgz should probably be @types-aria-query-aria-query-4.2.2.tgz, etc.. I'll look into this. I think related to old assumption that @scope was sufficient and there was no additional nesting.

Nykseli commented 1 year ago

I'll look into this.

Thanks!

I think related to old assumption that @scope was sufficient and there was no additional nesting.

Node packages are full of surprises :smile:

AdamMajer commented 1 year ago

This problem should be now fixed. I'm now generating a more unique filename by incorporating parts of the URL path to form the filename stored in the cpio archive. Would be awesome if someone can verify there are no more issues here.

Nykseli commented 1 year ago

Looks like it's still not fully working. I updated the node module archive using node_modules.py from https://github.com/openSUSE/obs-service-node_modules/commit/1526eaa237032a4eefb7c540c02a9e40e99937aa and it's still not able to build correctly. It still gives the following error:

[   17s] npm ERR! code ETARGET
[   17s] npm ERR! notarget No matching version found for aria-query@^4.2.2.
[   17s] npm ERR! notarget In most cases you or one of your dependencies are requesting
[   17s] npm ERR! notarget a package version that doesn't exist.

You can see the full log here: https://build.opensuse.org/package/live_build_log/home:malikirri/cockpit-wicked-playground/openSUSE_Tumbleweed/x86_64

The aria-query library is saved to the archive but looks like it's not able to find it there for some reason

$ cpio -t < node_modules.obscpio | grep aria-query
@testing-library-@types-aria-query-4.2.2.tgz
@testing-library-aria-query-4.2.2.tgz
@types-@types-aria-query-5.0.1.tgz
aria-query-5.1.3.tgz
279239 blocks
AdamMajer commented 1 year ago

The error is from npm.. Maybe there is some issue with the https://github.com/openSUSE/npm-localhost-proxy . I'll take a look

AdamMajer commented 1 year ago

looks like somehow @testing-library-aria-query-4.2.2.tgz is same as @testing-library-@types-aria-query-4.2.2.tgz ... re-opening

AdamMajer commented 1 year ago

Now it seems to be fixed :-) I can build the test project successfully. There was some old code that tried to generate unique filenames by using install paths but this somehow ended up overwriting the other modules... I'm actually uncertain how this ended up broken as the filename should still be unique and it's strange why wrong module is saved double under wrong filename... any ideas?

AdamMajer commented 1 year ago

What happened was the old version of the code wrote the testing-library-aria-query-4.2.2.tgz from the @types/aria-query-4.2.2.tgz archive. The new version saw this file as originating from aria-query-4.2.2.tgz and never bother to replace it or check further information, like size, etc.. (algorithm only checks if the file exists in archive!). This is the offending section is here:

https://github.com/openSUSE/obs-service-node_modules/blob/master/node_modules.py#L454

I think this may merit a new issue for improvement.

Mystery solved!

Nykseli commented 1 year ago

I was able to build the test project successfully too. Thank you for figuring it out!