Add user@address:port support#3425
Conversation
|
Not really opposed to this, but it's a lot of code to support something that can already be achieved via |
356b8ba to
081ad63
Compare
|
I'll move the additional debug logs to another PR Right now just IPv6 support is missing Meanwhile I learned how to C++, so that's why that took so long |
|
I marked this as stale due to inactivity. → More info |
|
I closed this issue due to inactivity. → More info |
|
Still interested |
|
There is already code for compression and SSH key, both of them can also be set in |
|
Note if we do this, we should also consider doing it with
from NixOS/hydra#1164 This syntax would be used in the build machines files too, and we don't want that to drift further apart between Nix and Hydra. |
|
Currently I've got no intention of updating this pr, so feel free to force push / make a new one (I should mention again I barely know c++ so it would be a pita for me to code this to the end anyways) |
|
OK, very understandable after 3 years! I converted it to a draft. |
|
Discussed last Nix team meeting. Verdict: Idea sounds good. Port parsing however should be pat of the URL parser in general, not extracted in this special case. Stores that don't support ports can just fail on that; stores that do like this one can set it very easily. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2023-04-10-nix-team-meeting-minutes-47/27357/1 |
2853843 to
9a84b87
Compare
2fd6b51 to
d293941
Compare
This patch allows users to specify the connection port in the store URLS like so: ``` nix store info --store "ssh-ng://localhost:22" --json ``` Previously this failed with: `error: failed to start SSH connection to 'localhost:22'`, because the code did not distinguish the port from the hostname. This patch remedies that problem by introducing a ParsedURL::Authority type for working with parsed authority components of URIs. Now that the URL parsing code is less ad-hoc we can add more long-awaited fixes for specifying SSH connection ports in store URIs. Builds upon the work from bd1d2d1. Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo> Co-authored-by: John Ericson <John.Ericson@Obsidian.Systems>
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
|
FYI: this appears to have broken EDIT: only when using a host like |
| oss << authority.host; | ||
| return std::move(oss).str(); | ||
| }()) | ||
| , fakeSSH(authority.host == "localhost") |
There was a problem hiding this comment.
@cole-h, this will make it behave as it used to:
| , fakeSSH(authority.host == "localhost") | |
| , fakeSSH(authority.to_string() == "localhost") |
This patch allows users to specify the connection port
in the store URLS like so:
Previously this failed with:
error: failed to start SSH connection to 'localhost:22',because the code did not distinguish the port from the hostname. This
patch remedies that problem by introducing a ParsedURL::Authority type
for working with parsed authority components of URIs.
Now that the URL parsing code is less ad-hoc we can
add more long-awaited fixes for specifying SSH connection
ports in store URIs.
Builds upon the work from bd1d2d1.
Fixes #7044.