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

Copy module folders always to temporary folder #1492

Merged
merged 1 commit into from
Jan 30, 2022
Merged

Conversation

trippier1
Copy link
Contributor

We have an issue reported by several users showing error message “Error: The specified module could not be found.” where some .node files cannot be found in Temp folder under Windows. Because of this error, application cannot be started.
After analysis we have seen that the directories inside temp remain but files are cleaned up (we don’t know which clean up mechanism, Windows OS or antivirus software but we anyway have no means to intervene here). pkg currently only checks for existence of the folder pkg/ and if exists, no copying is done. Since one can never be sure about what happens with files and directories in temporary folder when the application is not active, safer way would be to always do the copying.

@btsimonh
Copy link

Note: I don't know where the files go in pkg, but in my independent implementation of something similar, I now extract the files to folders other than temp. I never had an automatic cleanup issue on Windows, but did on both linux and OSX. I now use the 'programdata' folder equivalents on all platforms to avoid this.

@trippier1
Copy link
Contributor Author

@jesec thanks for the review. Can you please also approve running workflow?

@robertsLando
Copy link
Contributor

@jesec I think this is good to merge

@robertsLando
Copy link
Contributor

@trippier1 Another possible fix would be to allow providing tmp folder using an env var like PKG_TMP_FOLDER. This should allow users to place node addons files where they prefer

@jesec
Copy link
Contributor

jesec commented Jan 28, 2022

@trippier1 Another possible fix would be to allow providing tmp folder using an env var like PKG_TMP_FOLDER. This should allow users to place node addons files where they prefer

Ugh. We don't need more options, especially not the runtime env variables.

@jesec
Copy link
Contributor

jesec commented Jan 28, 2022

A CI run failed. I requeued it. Let's see how things go.

@robertsLando
Copy link
Contributor

robertsLando commented Jan 28, 2022

I don't mean a runtime env var that we need to bake into the executable, it could be just an env var user can set from executable if he wants

@jesec
Copy link
Contributor

jesec commented Jan 30, 2022

I don't mean a runtime env var that we need to bake into the executable, it could be just an env var user can set from executable if he wants

That's even worse. A baked in option is for the users, while a runtime env variable is for the end-users. We have to be much much more careful on options to provide to the end-users. So absolutely no.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants