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

Repository addresses must not contain the .git suffix #566

Closed
0x2b3bfa0 opened this issue May 26, 2021 · 4 comments · Fixed by #567 or #570
Closed

Repository addresses must not contain the .git suffix #566

0x2b3bfa0 opened this issue May 26, 2021 · 4 comments · Fixed by #567 or #570
Assignees
Labels
bug Something isn't working

Comments

@0x2b3bfa0
Copy link
Member

0x2b3bfa0 commented May 26, 2021

The error on #562 happened because the user configured a repository address with the .git suffix (e.g. https://github.com/user/repository.git) as the RUNNER_REPO / --repo option. It would be nice to clarify this somewhere in a troubleshooting section, if we plan to have it.

Desiderata, part of #514

@0x2b3bfa0 0x2b3bfa0 added the documentation Markdown files label May 26, 2021
@DavidGOrtega
Copy link
Contributor

Actually we do a cleanup of '.git' when inferring the repo. Should we apply it always?

@0x2b3bfa0
Copy link
Member Author

@DavidGOrtega, this only happens in the cases where the repository address is inferred from the Git repository itself, but not when it comes from the RUNNER_REPO environment variable or the --repo option. See #562 for a practical example.

cml/src/cml.js

Line 77 in 498fcd1

this.repo = uri_no_trailing_slash(repo || git_remote_url());

cml/src/cml.js

Lines 30 to 36 in 498fcd1

const git_remote_url = (opts = {}) => {
const { remote = GIT_REMOTE } = opts;
const url = execSync(`git config --get remote.${remote}.url`).toString(
'utf8'
);
return strip_auth(git_url_parse(url).toString('https').replace('.git', ''));
};

If we can safely remove the suffix from every address, let's do it in the code. I can't think of any case where we shouldn't remove it, but I'm not completely sure.

@0x2b3bfa0 0x2b3bfa0 added bug Something isn't working and removed documentation Markdown files labels May 27, 2021
@0x2b3bfa0
Copy link
Member Author

We can choose to:

  1. Keep the current logic and document the issue
  2. Remove the suffix automatically in every case

I find the latter rather convenient, but I still don't know if there is any special case[citation required] where this behavior can produce unexpected results. It seems pretty safe to me.

@DavidGOrtega
Copy link
Contributor

We can choose to:

  1. Keep the current logic and document the issue
  2. Remove the suffix automatically in every case

I find the latter rather convenient, but I still don't know if there is any special case[citation required] where this behavior can produce unexpected results. It seems pretty safe to me.

Agree. I can't see anything bad with it.

@0x2b3bfa0 0x2b3bfa0 changed the title Clarify that repository addresses must not contain the .git suffix Repository addresses must not contain the .git suffix May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants