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

Use nixpkgs#bashInteractive for dev-shell #3565

Merged
merged 5 commits into from May 28, 2020

Conversation

@mkenigs
Copy link
Contributor

mkenigs commented May 5, 2020

Instead of using system bash for dev-shell, uses nixpkgs#bashInteractive as discussed in #3107 (comment). If dev-shell is used for a flake with a toplevel nixpkgs input, it uses that nixpkgs per #3107 (comment).

Reused a lot of code from InstallableFlake - seems like that should maybe get factored out?

@matthewbauer

@@ -281,6 +284,40 @@ struct CmdDevShell : Common, MixEnvironment
};
}

std::string getBashPath(ref<Store> store)

This comment has been minimized.

Copy link
@edolstra

edolstra May 6, 2020

Member

Maybe this function can be generalized to fetch any package. For example, it could be used by the Git fetcher to fetch nixpkgs#git it git is not installed. Although that's a bit tricky right now because libfetcher doesn't have access to libexpr.

This comment has been minimized.

Copy link
@mkenigs

mkenigs May 6, 2020

Author Contributor

Could I start by taking out the SourceExprCommand from InstallableValue? Looks like all it's used for is state, getAutoArgs, and lockFlags, and it might be simpler for an Installable to be separate from a command. Then Installables could be used by InstallableCommands/fetchGit/dev-shell.

src/nix/installables.cc Outdated Show resolved Hide resolved
@mkenigs mkenigs mentioned this pull request May 9, 2020
@mkenigs mkenigs force-pushed the mkenigs:nixpkgs#bashInteractive branch from b7bd59e to ba7d7ed May 16, 2020
@mkenigs
Copy link
Contributor Author

mkenigs commented May 16, 2020

Updated to just create a bashInteractive InstallableFlake. InstallableFlake will only call lockFlake once, but this will ultimately still call lockFlake twice, because the bashInteractive installable will call it a second time.

Seems like preventing that would require being able to have a LockedFlake return a LockedFlake for one of its inputs, but not sure if that's possible right now. And then everything used by toStorePath might need to get moved to LockedFlake or somewhere in libexpr.

@domenkozar domenkozar requested a review from edolstra May 17, 2020
@edolstra edolstra merged commit 04fb4e8 into NixOS:flakes May 28, 2020
0 of 2 checks passed
0 of 2 checks passed
tests (ubuntu-18.04) tests (ubuntu-18.04)
Details
tests (macos)
Details
@edolstra
Copy link
Member

edolstra commented May 28, 2020

Thanks, merged! I changed nixpkgsFlakeRef() to use nixpkgs from lockedFlake->lockFile, since otherwise it will not necessarily use the same nixpkgs as what the top-level flake use.

@mkenigs mkenigs deleted the mkenigs:nixpkgs#bashInteractive branch May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.