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

rewrite long prefix paths as $PREFIX, etc. in stdout #3258

Merged
merged 5 commits into from
Nov 14, 2018

Conversation

minrk
Copy link
Contributor

@minrk minrk commented Nov 13, 2018

makes build outputs more readable, easier to follow

I’m not 100% sure if this is a good idea, but it greatly improves the experience of debugging conda-build output for me, at least. By using environment variables, the results are still copy/pasteable for replication and testing.

Still need to confirm that the use of pipes/rewrites is correct on Windows

Copy link
Contributor

@msarahan msarahan left a comment

Choose a reason for hiding this comment

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

This is a great idea! Thanks! I think we need to fix up Windows, but otherwise this looks good.

tests/test_build.py Outdated Show resolved Hide resolved
tests/test_build.py Outdated Show resolved Hide resolved
@mingwandroid
Copy link
Contributor

Great idea. I wanted this for a while.

@mingwandroid
Copy link
Contributor

It also allows running stuff pasted from logs (provided you run your activate) which is super handy.

@msarahan
Copy link
Contributor

@minrk let us know if you want us to finish this one for you. I'm not sure how much access to a Windows machine you have.

makes build outputs more readable

I’m not sure if this is a good idea, but it greatly improves
the experience of debugging conda-build output for me

Need to confirm that the use of pipes/rewrites are correct on Windows
@minrk
Copy link
Contributor Author

minrk commented Nov 14, 2018

Help finishing up would be awesome. I've been using this locally on and off for a while and it's been pretty nice, especially with builds that use lots of prefix-referencing compiler flags. In particular, I'm less confident about using pipes on Windows in general because I know that full pipes causing subprocess hangs can be an issue, which could be mysterious and incredibly frustrating.

I pushed a little more based on your feedback, but if folks could help get Windows right, that would be terrific.

@msarahan
Copy link
Contributor

Thanks. I will work on it today. In the meantime, any feedback you can provide on #3237 would be very helpful.

@@ -182,7 +182,7 @@ def test_create_info_files_json_no_inodes(testing_workdir, testing_metadata):


def test_rewrite_output(testing_workdir, testing_config, capsys):
api.build(os.path.join(metadata_dir, "rewrite_env"), config=testing_config)
api.build(os.path.join(metadata_dir, "_rewrite_env"), config=testing_config)
Copy link
Contributor

Choose a reason for hiding this comment

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

I moved this to _rewrite_env because it is a test that we want to manually run. Things starting with _ are not run automatically with https://github.com/conda/conda-build/blob/master/tests/test_api_build.py#L82-L94

@msarahan
Copy link
Contributor

Going in. Thanks @minrk!

@msarahan msarahan merged commit 20286a7 into conda:master Nov 14, 2018
@minrk minrk deleted the echo-prefix branch November 21, 2018 15:44
@github-actions
Copy link

Hi there, thank you for your contribution!

This pull request has been automatically locked because it has not had recent activity after being closed.

Please open a new issue or pull request if needed.

Thanks!

@github-actions github-actions bot added the locked [bot] locked due to inactivity label Apr 17, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked [bot] locked due to inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants