Skip to content

Conversation

Julien-R44
Copy link
Member

@Julien-R44 Julien-R44 commented Nov 19, 2023

So again ahah, we have a problem with the metafiles copying : if the lockfile does not exist, copying the meta files will fail. To recap, here's the problem I encountered:

  • I tried to build my Adonis project
  • The metafiles were not copied, even the package.json file was missing.
  • I investigated and noticed that it was trying to copy package-lock.json to the build folder. However, I'm using pnpm so I don't have a package-lock.json file.

The problem is that we copy the metafiles + package.json + lockfile in a single call to cpy: https://github.com/adonisjs/assembler/blob/fix/metafiles-cpy/src/bundler.ts#L223

Then we ignore ENOENT errors:
https://github.com/adonisjs/assembler/blob/fix/metafiles-cpy/src/bundler.ts#L105

And, when trying to copy metafiles + package.json + lockfile, since the lock file is missing, cpy will just throw an error and not copy anything.

So i am not sure how we should handle this, hence my PR with just a test that showcase the error.
I've opened a PR to the core that adds automatic detection of the package manager, which help to avoiding this error

Also, I wonder if we shouldn't find an alternative to cpy given the message added to their README: https://github.com/sindresorhus/cpy/releases/tag/v11.0.0

This package has a lot of problems and I unfortunately don't have time to fix them. I would recommend against using this package until these problems are resolved. Help welcome (see the issue tracker).

Feel free to give any directive and I can implement a fix

@thetutlage
Copy link
Member

How about we then drop cpy and use our custom implementation? We need a way to expand the glob pattern and then copy files maintaining the directory structure. I think, it shouldn't be that hard

@Julien-R44
Copy link
Member Author

Yes, agree. Let's do that

@Julien-R44
Copy link
Member Author

Should be all good

@Julien-R44 Julien-R44 marked this pull request as ready for review November 25, 2023 20:03
@RomainLanz
Copy link
Member

You still have conflicts, seems you have to rebase

@Julien-R44
Copy link
Member Author

what the fuck did I do

@Julien-R44
Copy link
Member Author

Okay looks like we can't rebase this PR, but we can squash it. Idk what the hell i've done
Let's squash it

@thetutlage thetutlage merged commit f3e3070 into next Nov 28, 2023
@thetutlage thetutlage deleted the fix/metafiles-cpy branch November 28, 2023 05:44
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

Successfully merging this pull request may close these issues.

3 participants