Add support for using ssh cloning with GitLab #1057
Conversation
|
|
Webrevs
|
| "\n"; | ||
|
|
||
| try { | ||
| Files.writeString(cfgFile, result + filtered.strip() + "\n", StandardCharsets.UTF_8); |
There was a problem hiding this comment.
Can this result in us writing to a users .ssh/config when running Skara CLI tools?
There was a problem hiding this comment.
No, CLI tools never pass a configuration object when creating a Forge.
| var existingBlock = Pattern.compile("^Match host=" + Pattern.quote(userHost) + "(?:\\R[ \\t]+.*)+", Pattern.MULTILINE); | ||
| var existingMatcher = existingBlock.matcher(existing); | ||
| var filtered = existingMatcher.replaceAll(""); | ||
| var result = "Match host=" + userHost + "\n" + |
There was a problem hiding this comment.
I don't think this is valid .ssh/config syntax. Afaik it should be Match <keyword> <value>, that is, no =. I see no tests in this PR. Are you sure this works?
There was a problem hiding this comment.
Yes, tested it manually and seems to work fine. And no, there are no automated tests that exercise GitLab or GitHub APIs, except for running them with a special credential configuration..
There was a problem hiding this comment.
I still think this does not follow the documented ssh config file syntax (https://man7.org/linux/man-pages/man5/ssh_config.5.html). And even if that works on your machine right now, it can break on a future update of ssh.
There was a problem hiding this comment.
Configuration options may be separated by whitespace or optional whitespace and exactly one ‘=’
Still, works without it as well, so can certainly change it..
|
@rwestberg This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 2 new commits pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the
|
|
/integrate |
|
@rwestberg Since your change was applied there have been 3 commits pushed to the
Your commit was automatically rebased without conflicts. Pushed as commit 2ca0412. |
Add support for configuring a GitLab forge to use ssh for cloning repositories using a private ssh key.
Progress
Reviewers
Download
$ git fetch https://git.openjdk.java.net/skara pull/1057/head:pull/1057$ git checkout pull/1057