-
Notifications
You must be signed in to change notification settings - Fork 371
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
Delay the creation of the context used when a package failed to build up until the package failed #6278
base: master
Are you sure you want to change the base?
Conversation
33d9210
to
c0bcb68
Compare
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.
The PR contains 2 things: the delay of context creation for package build, and the delay of metadata processes as the sumary is not always printed. PR title is good, but main comment and master changes should contains both. Ideally, 2 commits if it can be easily split.
Otherwise, lgtm!
… up until the package failed
c0bcb68
to
c73333f
Compare
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.
The PR contains 2 things: the delay of context creation for package build, and the delay of metadata processes as the sumary is not always printed. PR title is good, but main comment and master changes should contains both. Ideally, 2 commits if it can be easily split.
I don't think those are two separate things. cmd_metadata
, p_metadata
and r_info
are all storing the same things only in three different types. If anything i should be talking about metadata instead of context
@@ -388,7 +395,9 @@ let create ?info_file ?env_file ?(allow_stdin=not Sys.win32) ?stdout_file ?stder | |||
| Some f -> | |||
let chan = open_out f in | |||
let info = | |||
make_info ~cmd ~args ~cwd ~env_file ~stdout_file ~stderr_file ~metadata () in | |||
make_info ~cmd ~args ~cwd ~env_file ~stdout_file ~stderr_file | |||
~metadata:(Lazy.force metadata) () |
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 forced too early. I think there is a way to run it only when the process finished
Noticed while reviewing #6274.
It turns out that we try to fetch the revision of each packages being built even before executing the command, but that information is only used in rare cases so if many git packages are used this can cause some wasted time (not too much, up to a couple seconds on slow hardware with large installs, but not nothing)