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

Fix next prompt detector in generate_precompile_statements #44196

Merged

Conversation

IanButterworth
Copy link
Sponsor Member

@IanButterworth IanButterworth commented Feb 16, 2022

In generate_precompile_statements it turns out the repeatedly overprinted > chars from the Pkg.precompile progress bar were being interpreted as the next prompts.. making the precompile statement generator process race to the end and crashing the end of the process. I'm not sure whether that means the scripts afterwards failed to get precompile statements generated.

This was confirmed by setting debug_output = stdout

@IanButterworth IanButterworth added backport 1.6 Change should be backported to release-1.6 backport 1.7 labels Feb 16, 2022
@IanButterworth
Copy link
Sponsor Member Author

Judging by the previous commit it seems precompile statements were being missed off..

On the MacOS buildbot

Previous commit:

Executing precompile statements... 1992/2036

This PR:

Executing precompile statements... 2017/2061

@KristofferC KristofferC added the backport 1.8 Change should be backported to release-1.8 label Feb 16, 2022
@KristofferC
Copy link
Sponsor Member

Nice

occursin(PKG_PROMPT, strbuf) && break
occursin(SHELL_PROMPT, strbuf) && break
occursin(HELP_PROMPT, strbuf) && break
sleep(0.1)
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Why is this sleep needed?

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Oh, I assume that readavailable blocks on no data but that might not be the case?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

Maybe it isn't, I didn't test it without. I just thought spinning without wait if the terminal is stalling output wasn't necessary

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Maybe we need a readuntil function that takes a regex

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

yeah, I looked for that while doing this

@KristofferC KristofferC merged commit c839221 into JuliaLang:master Feb 16, 2022
KristofferC pushed a commit that referenced this pull request Feb 16, 2022
@KristofferC KristofferC mentioned this pull request Feb 16, 2022
7 tasks
@IanButterworth IanButterworth deleted the ib/gen_precompile_promp_fix branch February 16, 2022 13:05
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Feb 17, 2022
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request Feb 17, 2022
KristofferC pushed a commit that referenced this pull request Feb 19, 2022
@KristofferC KristofferC mentioned this pull request Feb 19, 2022
50 tasks
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
KristofferC pushed a commit that referenced this pull request Feb 23, 2022
@KristofferC KristofferC mentioned this pull request Feb 23, 2022
40 tasks
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label May 16, 2022
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
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

3 participants