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

Danger inadvertently making the git repository shallow [teamcity] [bitbucket cloud] #1314

Open
acecilia opened this issue Jul 30, 2021 · 6 comments

Comments

@acecilia
Copy link
Member

acecilia commented Jul 30, 2021

Report

What did you do?

Run Danger on bitbucket cloud + teamcity. Teamcity makes full clones (not shallow) of the PRs

What did you expect to happen?

Running danger does not alter the git repository

What happened instead?

Danger converted the git repository into a shallow clone

Your Environment

  • Which CI are you running on?
    Teamcity
  • Are you running the latest version of Danger?
    Yes, I am running 8.3.1
  • What is your Dangerfile?
    puts "is shallow:"
    system('git rev-parse --is-shallow-repository')
    puts "done"

How does this happen?

I have been troubleshooting the issue for a while. The issue is difficult to reproduce, and it could look like it is aleatory, but it is not. I think this is what is happening:

Scenario 1 [expected]: the git repository stays as full clone (not shallow)

  • A PR branches from commit commit-1-in-master, and adds a commit commit-1-in-branch
  • When the PR is opened, danger runs on CI
  • When Danger runs, it requests the PR information from bitbucket cloud. The response contains the following information:
    • Branch last commit: commit-1-in-branch
    • Target branch last commit (master): commit-1-in-master
  • With that information, danger runs. It will call the function commit_exists in here which returns true: the full clone done by teamcity contains both commits commit-1-in-branch and commit-1-in-master.
  • The execution of the Dangerfile results in the following output:
is shallow:
false
done

Scenario 2 [unexpected]: the repository gets converted to shallow
After a while, master branch gets new commits: commit-2-in-master, commit-3-in-master, commit-4-in-master... And now danger behaves differently. See below scenario:

  • The already opened PR gets a second commit commit-2-in-branch
  • CI detects it, and triggers a new danger run
  • When Danger runs, it requests the PR information from bitbucket cloud. The response contains the following information:
    • Branch last commit: commit-2-in-branch
    • Target branch last commit (master): commit-4-in-master
  • With that information, danger runs. It will call the function commit_exists in here which returns false for the commit-4-in-master: such commit is not part of the clone done by teamcity! The clone only contains commit-1-in-branch, commit-2-in-branch and commit-1-in-master
  • As a result, danger runs the function git_fetch_branch_to_depth here, which converts the repository to shallow (is this known, is it expected?)
  • The execution of the Dangerfile results in the following output:
is shallow:
true
done

How to troubleshoot it?

Not so easy. I end up modifying the danger gem in place and added some prints:
Screenshot 2021-07-30 at 13 40 12
Screenshot 2021-07-30 at 13 42 53

Under scenario 2, I get the following output in my CI (I replaced real commit hashes with their equivalent based on above explanation):

  ensure_commitish_exists_on_branch1
  commit-4-in-master
  ensure_commitish_exists_on_branch2
  git_fetch_branch_to_depth
  From bitbucket.org:my_org/my_repo
     commit-1-in-master..commit-4-in-master  master -> origin/master
  ensure_commitish_exists_on_branch1
  commit-2-in-branch
  ensure_commitish_exists_on_branch1
  danger_base
  ensure_commitish_exists_on_branch1
  danger_head

Why is this a problem?

  • Because it is very unexpected to change the repository to a shallow clone. If CI invests the time to make a full clone, it expects the full clone to be there through the run
  • Because sometimes CI relies on having the full branch history available. For example: if you want to check on CI that a branch contains a specific commit, in case the commit happened before commit-1-in-master:
    • Before running danger, doing git branch "branch" --contains "commit-0-in-master" returns the current branch, as expected
    • After running danger (which converted the repo to a shallow clone), doing git branch "branch" --contains "commit-0-in-master" does not return the current branch

What could be a solution?

  • Do not alter the repository in place, and instead do it in a temporary copy of the repository
  • Alter the repository in place in order to be able to calculate the diff, but then rollback the changes and leave the repository as it was (not sure if possible)

Workaround

Run git fetch origin master before running danger, so all commits needed by danger are already in the clone, avoiding the call to the git_fetch_branch_to_depth function

@orta
Copy link
Member

orta commented Jul 31, 2021

Perhaps git_fetch_branch_to_depth can check for an existing depth to see if it is longer than one, and if so avoid calling prune?

@angelolloqui
Copy link

angelolloqui commented Sep 19, 2023

Hi, what is the status of this issue? I have a deep git history and running danger is adding around 2min extra just for the shallow copy, for running some scripts that should not need any extra info that the already contained in the current checkout. Any way to force activating/deactivating the shallow? thanks!

@orta
Copy link
Member

orta commented Sep 19, 2023

You can switch to Danger JS which only uses the source control APIs to handle it - but Danger in ruby requires both base and head in the git repo to run

@acecilia
Copy link
Member Author

acecilia commented Sep 19, 2023

Or use the workaround mentioned in the description. I’m using it since I opened this issue

@angelolloqui
Copy link

My problem is that I want to avoid the git fetch because it takes 3 min extra due to the enormous history, and does it for no real reason. Thanks anyway

@acecilia
Copy link
Member Author

What about if you limit git fetch to the target branch. You do not need to fetch all, only target branch

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

No branches or pull requests

3 participants