Skip to content

chore: change rmdir to rm #35

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jlucaso1
Copy link

@jlucaso1 jlucaso1 commented Jun 10, 2025

Hey there!
When running pnpm build on a fresh clone, the build fails because Cargo cannot fetch the ext-php-rs dependency. This happens because the dependency is referenced with an SSH URL, which requires authentication.

error: failed to get `ext-php-rs` as a dependency ...
Caused by:
  Unable to update ssh://[email protected]/platformatic/ext-php-rs.git
Caused by:
  failed to authenticate when downloading repository

Extra: removed deprecated rmdir from test/util.mjs

@jlucaso1 jlucaso1 changed the title fix: Replace deprecated rmdir with rm in MockRoot clean method fix: Use HTTPS for ext-php-rs git dependency to fix local build Jun 10, 2025
@Qard
Copy link
Member

Qard commented Jun 11, 2025

Yep, that was necessary when the repo was private for the CI to be able to pull it, but I think that should not be necessary anymore.

Also, rmdir should not be deprecated. Did you get a warning from it?

@jlucaso1
Copy link
Author

jlucaso1 commented Jun 11, 2025

Also, rmdir should not be deprecated. Did you get a warning from it?

Yes. i'm getting the same as nrwl/nx#7739
Im using latest nodejs version

@Qard Qard closed this in #37 Jun 11, 2025
@Qard
Copy link
Member

Qard commented Jun 11, 2025

The dependency has been fixed with #37. There was some additional CI changes which needed to happen alongside it, so I just reproduced the PR.

@Qard
Copy link
Member

Qard commented Jun 11, 2025

If you want to update this PR to get rid of the dep change, I can merge this to fix the rmdir warning. Otherwise, I can reproduce that myself too, if you prefer. 🙂

@Qard Qard reopened this Jun 11, 2025
@jlucaso1 jlucaso1 force-pushed the fix/build-dependency-url branch from d0e2c0f to 22fbb3e Compare June 11, 2025 14:58
@jlucaso1
Copy link
Author

Done 😊. Thank you

@jlucaso1 jlucaso1 changed the title fix: Use HTTPS for ext-php-rs git dependency to fix local build chore: change rmdir to rm Jun 11, 2025
@Qard
Copy link
Member

Qard commented Jun 11, 2025

I'll see about rebasing this tomorrow after #39 lands. I missed that before. Once that's in there the lint should pass properly. 🙂

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.

2 participants