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

very simple fix for #309 #335

Merged
merged 1 commit into from
Aug 3, 2015
Merged

Conversation

eddelbuettel
Copy link
Member

if no rebuild is needed, still set an on.exit() handler to get back to the
starting directory

if no rebuild is needed, still set an on.exit() handler to get back to the
starting directory
@jjallaire
Copy link
Member

Let me take a closer look at this before we merge. From first glance it appears that we haven't yet done the setwd in the case of the else clause you put in however clearly it's happening somewhere (perhaps on the native code side?). Want to make sure I fully understand the source of the bug before we apply the fix.

@eddelbuettel
Copy link
Member Author

Sure. I instrumented a local copy with simply i/o to see which setwd() was being triggered (which I'll email you in a minute). That made it pretty apparent that we lacked one in the 'no rebuild' case. But by all means take a closer look; my analysis was clearly of the Sunday-morning variety :)

jjallaire added a commit that referenced this pull request Aug 3, 2015
@jjallaire jjallaire merged commit d680346 into master Aug 3, 2015
@jjallaire jjallaire deleted the bugfix/setwd-on-exit-in-sourceCpp branch August 3, 2015 10:03
@jjallaire
Copy link
Member

The fix was as expected correct (I had missed that we also set the working directory as when sourcing embedded R code). I was puzzled why we might not have seen this more often in the field but it requires the combination of:

  1. Sourcing a file from outside the current directory more then once; and
  2. The presence of an embedded R code chunk in the file.

Certainly plausible that this doesn't happen all that often.

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.

None yet

2 participants