Keep splitting libcmd headers & files#7748
Conversation
Ericson2314
changed the title
|
|
||
| *.a | ||
| *.o | ||
| *.o.tmp |
There was a problem hiding this comment.
Oh oops sorry this snuck in here. Clang build temp files like this, which I noticed when I started developing with clang, and so I added this line while working. They are deleted by Clang after the build is complete, but if you run git status while a build is in progress (say in another terminal), they clutter the output.
I had meant to PR this separately; let me know if I still should.
|
🎉 All dependencies have been resolved ! |
| } | ||
|
|
||
| Installables InstallablesCommand::load() { | ||
| Installables installables; |
There was a problem hiding this comment.
This variable was unused; surprised there was no warning!
| if (_installables.empty()) { | ||
| if (useDefaultInstallables()) | ||
| return {"."}; | ||
| return {}; |
There was a problem hiding this comment.
Since _installables is empty, this is the same as return _installables; below.
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
|
|
||
| libcmd_SOURCES := $(wildcard $(d)/*.cc) | ||
|
|
||
| libcmd_CXXFLAGS += -I src/libutil -I src/libstore -I src/libexpr -I src/libmain -I src/libfetchers -I src/nix |
There was a problem hiding this comment.
This layer violation is no longer needed after moving these things around.
There was a problem hiding this comment.
The issue was the #include "repl.md", which was left behind in src/nix not moved to src/libcmd. It could of been moved over, but this PR solves it by moving (the CmdRepl part of) repl.cc back.
|
Discussed on the Nix team meeting:
|
The REPL itself and the `nix repl` CLI are conceptually different things, and thus deserve to be in different files.
Ericson2314
force-pushed
the
split-other-installables
branch
from
7a85980 to
1bd03ad
Compare
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
|
This broke the noGC builds: https://hydra.nixos.org/build/210107828 And the aarch64-linux build is segfaulting: https://hydra.nixos.org/build/210107827 |
|
looking…. |
|
I know how to fix the first, I guess I will try to do a binfmt misc qemu GDB for the second. |
|
The aarch64-linux failure is not due to this but instead #6538 |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
5 participants
Motivation
More splitting things up for tidiness and/or NixOS/rfcs#134 sake.
Each commit can be understood in isolation:
Split out
InstallableFlakeandInstallableAttrPathPicking up where #7744 left off.
Slight cleanup of
InstallablesCommand::loadSee comments below, just removing dead code and deduplicate branches.
Split out
CmdReplandeditorForThe REPL itself and the
nix replCLI are conceptually different things, and thus deserve to be in different files.I also think
libcmdshould have the infrastructure for making the commands, but not the commands themselves.CmdRepl, previously there, not back inside the Nix binary with the other commands, would be one of those "commands themselves".An existing "obvious home" for
editorFordidn't pop into my mind, so I just gave it its own header and file.Context
Depends on #7744, see the PR's context for additional information.
Checklist for maintainers
Maintainers: tick if completed or explain if not relevant
tests/**.shsrc/*/teststests/nixos/*