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

Do not (re)use tags to find PRs #29

Open
rousskov opened this issue Mar 13, 2019 · 4 comments
Open

Do not (re)use tags to find PRs #29

rousskov opened this issue Mar 13, 2019 · 4 comments
Assignees

Comments

@rousskov
Copy link
Contributor

From squid-cache/squid#379 (comment)

I could not find in SemaphoreCI log the Git command causing the error. Anyway, I think we should configure it

I doubt we can configure it. I suspect that the git command is generated internally by Semaphore CI.

get the fresh copy to resolve this problem

I agree that, in principle, any internal repository caching done by Semaphore CI should not affect our builds -- a cache should be invisible to its users, especially users that do not have enough control over caching. In other words, Semaphore CI is at fault here.

However, according to the Jenkins issue you have found, Anubis should not be using tags [the way it uses them now]. Git tags are not supposed to change, but Anubis changes them.

I propose to get rid of Anubis PR tags completely. IIRC, they are only required to tie the staged commit to the PR. Since any commit not at the tip of the auto branch is irrelevant (stale/etc.), we can ignore those as if they did not exist. And we can tie the tip of the auto branch to PR NNN by using the last (#NNN) entry in the commit title.

Any objections or better ideas?

@eduard-bagdasaryan
Copy link
Contributor

Can we simply assign a new tag each time by appending a unique suffix? For example, M-staged-PR379-1, M-staged-PR379-2, ... or M-staged-PR379-t1, M-staged-PR379-t2, ...? I think it would be easier to adjust the code with it, avoiding vast architectural changes.

@rousskov
Copy link
Contributor Author

I have considered that option (before suggesting to get rid of tags) and rejected it because:

  1. adding suffixes to tags is not a trivial change
  2. polluting the official repository with even more tags is a step in the wrong direction; we are abusing tags -- the current design is conceptually wrong
  3. doing the right thing does not seem very complex and will simplify Anubis

We may disagree on the last item. I agree that it would be easier to adjust the code to add tag suffixes, but I do not think it would be easy enough to justify the time wasted on going in the wrong direction.

@eduard-bagdasaryan
Copy link
Contributor

polluting the official repository with even more tags

However, our usage of lightweight tags does not contradict the official 'git-tag' man page:

Annotated tags are meant for release while lightweight tags are meant for private
or temporary object labels. For this reason, some git commands for naming objects
(like git describe) will ignore lightweight tags by default.

So we correctly use them as 'temporary objects', I assume.

@rousskov
Copy link
Contributor Author

The notions of "tagging a temporary object" and "changing the tag value" are orthogonal. Yes, we had the right to tag staged commits. However, we should not have reused tag names, and (more importantly from architectural point of view) we should not have polluted the official git repository with our temporary housekeeping information.

If Anubis had a cloned repository, it would be OK to tag staged commits there (and never push them to the official repository). However, we are trying to avoid maintaining a local database (or equivalent).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants