Skip to content
This repository has been archived by the owner on May 9, 2024. It is now read-only.

Fix issue where 10updocker fails to add custom domain entries in /etc/hosts. #338

Merged
merged 3 commits into from
Jul 7, 2023

Conversation

desaiuditd
Copy link
Contributor

@desaiuditd desaiuditd commented Jun 19, 2023

Description of the Change

Seems like the code assumes that the file path for mkcertPrebuilt is a valid file system path. It's not always the case.

Sometimes (as is the case in #312) the Node is installed in a directory where there's a space in the directory name. Such paths with a space in-between will break the script, if not escaped properly.

As it's rightly pointed out #312 (comment).

I've escaped the shell binary path with double quotes so that unexpected space characters in the file path doesn't break the script.

Closes #312

How to test the Change

Not sure. Haven't followed any testing strategy here. But happy to follow any guidance here.

Changelog Entry

Fixed - Bug fix #312

Credits

Props ...

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.
Fix error when adding host file entries

```
Error: Command failed: /Users/admin/Library/Caches/fnm_multishells/2098_1687516908456/bin/node /Users/admin/Library/Application Support/fnm/node-versions/v18.16.0/installation/lib/node_modules/wp-local-docker/hosts.js add gus.local
node:internal/modules/cjs/loader:1078
  throw err;
  ^

Error: Cannot find module '/Users/admin/Library/Application'
    at Module._resolveFilename (node:internal/modules/cjs/loader:1075:15)
    at Module._load (node:internal/modules/cjs/loader:920:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:23:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

Node.js v18.16.0

    at /Users/admin/Library/Application Support/fnm/node-versions/v18.16.0/installation/lib/node_modules/wp-local-docker/node_modules/sudo-prompt/index.js:390:27
    at FSReqCallback.readFileAfterClose [as oncomplete] (node:internal/fs/read_file_context:68:3) {
  code: 1
}
⚠ Something went wrong adding host file entries. You may need to add the /etc/hosts entries manually.
```
Fix error when removing host file entries

```
Error: Command failed: /Users/admin/Library/Caches/fnm_multishells/2098_1687516908456/bin/node /Users/admin/Library/Application Support/fnm/node-versions/v18.16.0/installation/lib/node_modules/wp-local-docker/hosts.js remove gus.local
node:internal/modules/cjs/loader:1078
  throw err;
  ^

Error: Cannot find module '/Users/admin/Library/Application'
    at Module._resolveFilename (node:internal/modules/cjs/loader:1075:15)
    at Module._load (node:internal/modules/cjs/loader:920:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:23:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

Node.js v18.16.0

    at /Users/admin/Library/Application Support/fnm/node-versions/v18.16.0/installation/lib/node_modules/wp-local-docker/node_modules/sudo-prompt/index.js:390:27
    at FSReqCallback.readFileAfterClose [as oncomplete] (node:internal/fs/read_file_context:68:3) {
  code: 1
}
⚠ Something went wrong deleting host file entries. There may still be remnants in /etc/hosts
```
Copy link
Collaborator

@darylldoyle darylldoyle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, thanks for making the update!

@darylldoyle darylldoyle added this to the 4.0.0 milestone Jul 6, 2023
@darylldoyle darylldoyle merged commit de80abc into develop Jul 7, 2023
1 check passed
@darylldoyle darylldoyle deleted the desaiuditd-patch-1 branch July 7, 2023 07:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
2 participants