Skip to content
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

Make 'rc.path' absolute #158

Merged
merged 3 commits into from
Aug 11, 2021
Merged

Make 'rc.path' absolute #158

merged 3 commits into from
Aug 11, 2021

Conversation

dunkmann00
Copy link
Contributor

@dunkmann00 dunkmann00 commented Jun 23, 2021

I went ahead and implemented the suggestion by @vweevers. This will make rc.path an absolute path.

This fixes #157.

@vweevers
Copy link
Member

I would prefer to do it here:

prebuild-install/rc.js

Lines 45 to 47 in cca87fb

if (rc.path === true) {
delete rc.path
}

So that we don't modify rc outside of rc. Along the lines of:

rc.path = path.resolve(rc.path === true ? '.' : rc.path || '.')

@vweevers
Copy link
Member

Can you update this test accordingly?

t.equal(rc.path, '../some/other/path', 'correct path')
t.equal(rc.path, rc.p, 'path alias')

@dunkmann00
Copy link
Contributor Author

I just saw that after I pushed it, I'll take a look!

What should I set as the correct path? Running the tests on Github vs locally will result in a different value for path.

@vweevers
Copy link
Member

It's OK to use path.resolve('../some/other/path') in the test as well. Or more precisely (because the test uses a random temporary directory as the working directory): path.resolve(tmp, '../some/other/path'). One way to get that tmp variable is to pass it to the callback here:

cb(result)

I.e. cb(result, tmp)

test/rc-test.js Outdated Show resolved Hide resolved
@vweevers vweevers merged commit 57bcc06 into prebuild:master Aug 11, 2021
@vweevers
Copy link
Member

6.1.4. Thanks!

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.

--path option writing to wrong destination
2 participants