-
Notifications
You must be signed in to change notification settings - Fork 231
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
Fix/remove vm args inclusion restriction #832
Fix/remove vm args inclusion restriction #832
Conversation
if (system("test -r "file)) { | ||
print file" not readable" | ||
exit 3 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't we want these until OTP at least shows the errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we keep them, we'll deny any runtime vm.args
file generation that might be occurring, extended start script hooks allow you to do this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, i guess we can check if the file is readable only if it exists
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated, now the check for a readable file only happens when the file actually exists. With this change start script now tolerates included vm.args file that don't exist (yet).
while ((getline line<file)>0) { | ||
if (line~/^-args_file +/) { | ||
gsub(/^-args_file +| *$/, "", line) | ||
if (!(line~/^\//)) { | ||
print "relative path "line" encountered in "file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the actual change in this PR, right? It is to now allow relative paths in vm args?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, besides this it also removes the need for the included file to already exist when the start script runs, an allowed use case is having the start script hooks generate these (i'm actually going this route with a plugin i'm writing
3e7b9fb
to
eeebcf9
Compare
This is the red test that is failing for now.
OTP supports relative paths on the `-args_file` parameter, relx should not be blocking users from using this functionality.
eeebcf9
to
8e3d34c
Compare
No description provided.