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.

Sign up for GitHub

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

Jump to bottom

[main] bail out of activation if environment is not writable #1127

Merged
merged 4 commits into from
Jul 28, 2023
Merged

[main] bail out of activation if environment is not writable #1127

merged 4 commits into from
Jul 28, 2023

Conversation

h-vetinari
Copy link
Member

@h-vetinari h-vetinari commented Jul 28, 2023 •
edited
Loading

Reinstate 7c0287a

Closes #1126

Also pick up some small improvements from #1124

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@h-vetinari h-vetinari mentioned this pull request Jul 28, 2023
Closed
1 task
@maresb
Copy link
Contributor

maresb commented Jul 28, 2023

What's the purpose of the GDB wrappers in the first place? (Despite having contributed I don't actually understand what's going on. 😂)

@h-vetinari
Copy link
Member Author

What's the purpose of the GDB wrappers in the first place?

gdb needs to know where to look for symbols, and has a very specific path pattern it expects. Since that pattern depends on the exact path of the environment (which we cannot know at packaging time), the only option is to do something with an activation script (or not at all).

But where that approach is not possible, we might as well drop gdb support. It's not that important.

@maresb
Copy link
Contributor

maresb commented Jul 28, 2023 •
edited
Loading

Ok, so it's only if someone wants to use a debugger with libarrow? That doesn't sound so important.

Maybe we should modify the log message to say so?

Unless you plan to use the GDB debugger with libarrow, then this warning can be safely ignored.

@h-vetinari
Copy link
Member Author

Ok, so it's only if someone wants to use a debugger with libarrow? That doesn't sound so important.

Yes and yes.

Maybe we should modify the log message to say so?

OK

@h-vetinari h-vetinari force-pushed the readonly branch 5 times, most recently from d242749 to c15281f Compare July 28, 2023 15:11
@h-vetinari h-vetinari changed the title bail out of activation if environment is not writable [main] bail out of activation if environment is not writable Jul 28, 2023
@h-vetinari h-vetinari mentioned this pull request Jul 28, 2023
Merged
@h-vetinari h-vetinari mentioned this pull request Jul 28, 2023
Merged
@h-vetinari
Copy link
Member Author

Hm, I've never seen github take minutes between a force-push and it showing up in the PR. I did it a couple times to double-check... 🙈

@h-vetinari h-vetinari added the automerge Merge the PR when CI passes label Jul 28, 2023
maresb
maresb approved these changes Jul 28, 2023
View reviewed changes
Copy link
Contributor

@maresb maresb 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 great, thanks for this!

Maybe we want to also add something similar to ERROR: Failed to create symlink...?

@h-vetinari
Copy link
Member Author

I've switched off my laptop already. Perhaps the next time. ;)
Or feel free to submit a PR that switches the error to (visible by default) warning. 🙃

@maresb maresb mentioned this pull request Jul 28, 2023
Closed
@github-actions github-actions bot merged commit a32211f into conda-forge:main Jul 28, 2023
@github-actions
Copy link
Contributor

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

  • linter: passed
  • azure: passed

Thus the PR was passing and merged! Have a great day!

@h-vetinari h-vetinari deleted the readonly branch July 28, 2023 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maresb maresb maresb approved these changes

@cpcloud cpcloud Awaiting requested review from cpcloud cpcloud is a code owner

@jakirkham jakirkham Awaiting requested review from jakirkham jakirkham is a code owner

@jreback jreback Awaiting requested review from jreback jreback is a code owner

@kou kou Awaiting requested review from kou kou is a code owner

@kszucs kszucs Awaiting requested review from kszucs kszucs is a code owner

@leifwalsh leifwalsh Awaiting requested review from leifwalsh leifwalsh is a code owner

@nealrichardson nealrichardson Awaiting requested review from nealrichardson nealrichardson is a code owner

@pcmoritz pcmoritz Awaiting requested review from pcmoritz pcmoritz is a code owner

@pearu pearu Awaiting requested review from pearu pearu is a code owner

@pitrou pitrou Awaiting requested review from pitrou pitrou is a code owner

@robertnishihara robertnishihara Awaiting requested review from robertnishihara robertnishihara is a code owner

@siddharthteotia siddharthteotia Awaiting requested review from siddharthteotia siddharthteotia is a code owner

@wesm wesm Awaiting requested review from wesm wesm is a code owner

@xhochy xhochy Awaiting requested review from xhochy xhochy is a code owner

Assignees
No one assigned
Labels
automerge Merge the PR when CI passes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Activating read-only conda environment with arrow installed
2 participants
@h-vetinari @maresb