Skip to content

Spawn a new process for each Pkg.build #13499

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

Closed
wants to merge 1 commit into from

Conversation

stevengj
Copy link
Member

@stevengj stevengj commented Oct 8, 2015

This isolates the build process from the running environment, and prevents build failures due to stale modules already being loaded, as discussed in #13458.

@stevengj stevengj added the packages Package management and loading label Oct 8, 2015
@stevengj
Copy link
Member Author

stevengj commented Oct 8, 2015

Seems like an unrelated AppVeyor hiccup (fatal: unable to access 'https://github.com/JuliaLang/julia.git/': Could not resolve host: github.com). Will do a force push to restart the build.

warnbanner(err, label="[ ERROR: $pkg ]")
errs[pkg] = err
end
end
end
isfile(errfile) && Base.rm(errfile)
Copy link
Member

Choose a reason for hiding this comment

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

We should keep log file in case of an error for further inspection.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe, but that seems like a separate PR. We aren't keeping a log file now, so logging would be a new feature that would require its own design discussion.

The temporary file used here is poorly suited as a log file because I'm re-using the same temporary file for building each package.

Furthermore, the only information in this temporary file is a serialized exception, which is already deserialized and stored in the errs array, so I don't understand what would be gained by keeping the file around.

Copy link
Member

Choose a reason for hiding this comment

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

ok, a new feature is fine.

@stevengj
Copy link
Member Author

stevengj commented Oct 8, 2015

Will merge Friday if there are no objections.

@tkelman, this is probably worth backporting to 0.4.1; it should hopefully make Pkg.build more reliable, but doesn't otherwise affect the API.

@tkelman
Copy link
Contributor

tkelman commented Oct 9, 2015

This is going to slow things down a lot on Windows. Startup time is still around 3 seconds there. Can we do this as one process per call to Pkg.build, not one process per package?

@wildart
Copy link
Member

wildart commented Oct 9, 2015

Running in one process would defy whole idea to build a dependency in a clean environment.

@ScottPJones
Copy link
Contributor

Is workspace() not sufficient for this case, to have a clean environment?

@stevengj
Copy link
Member Author

stevengj commented Oct 9, 2015

One process per Pkg.build would solve most of the problems, I think, because things are built in dependency order, and are built only once.

I'm wary of workspace. It's not clear that I can restore the original Main when I am done, and I'm also wary of the workspace swap interacting with running tasks, e.g. imagine you do Pkg.build in IJulia where there are background tasks running.

@stevengj
Copy link
Member Author

stevengj commented Oct 9, 2015

Closing in favor of #13506.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages Package management and loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants