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

Revert "xonsh: set dontWrapPythonPrograms" #301449

Merged
merged 1 commit into from
Apr 5, 2024
Merged

Conversation

SamLukeYes
Copy link
Member

This reverts commit b640371.

Description of changes

A quick fix that resolves #301444

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@SamLukeYes
Copy link
Member Author

It shouldn't be necessary tho -- why do we need to re-introduce wrappers to xonsh-unwrapped 😕

@LordGrimmauld
Copy link
Contributor

LordGrimmauld commented Apr 4, 2024

running into this myself. Sadly the unwrapped xonsh isn't available by itself so overriding dontWrapPythonPrograms = false in an overlay is bothersome. Ideally xonsh-unwrapped would be available as a package itself....

The reason xonsh is wrapped in the first place is #240246, providing a way to add extra python packages that can be imported. The base package was pretty much kept without major changes that would allow it to not be wrapped at all.

@FlyingWombat
Copy link
Contributor

Sadly the unwrapped xonsh isn't available by itself so overriding...

Agreed, xonsh wrapper needs re-work. Since xonsh-unwrapped is unavailable, I can't build my own version from git in an overlay, only by editing nixpkgs.

@FlyingWombat
Copy link
Contributor

This changes builds and works for me.

Copy link
Contributor

@LordGrimmauld LordGrimmauld left a comment

Choose a reason for hiding this comment

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

Fix is needed. Seeing as you were the one removing the wrapping, i see no issue with you putting it back. But eventually xonsh-unwrapped should be available by itself and maybe less hacky too...

@LordGrimmauld LordGrimmauld added 6.topic: python 12.approvals: 1 This PR was reviewed and approved by one reputable person labels Apr 5, 2024
@wineee wineee merged commit 2188a1c into NixOS:master Apr 5, 2024
26 checks passed
@SuperSandro2000
Copy link
Member

Likely this is caused by #297628

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Xonsh no longer start
5 participants