Skip to content

fix(internal/glrepo/resolver): don't use pointers to gitlab.Project{}

Kerri Miller requested to merge github/fork/maxice8/pointers-ugh into trunk

Created by: maxice8

Description

Using pointers trigger a very interesting bug, imagine this case:

Remotes:

More important information:

  • Leo/aports is a fork of alpine/aports.

What happens ?

This piece of code runs, in this case r.network has the info from the 2 remotes.

for _, repo := range r.network {
    if repo.ForkedFromProject != nil {
        fProject, _ := api.GetProject(r.apiClient, repo.ForkedFromProject.PathWithNamespace)
        add(fProject)
    }
    add(&repo)
}

Here is the add function that will be referenced:

add := func(r *gitlab.Project) {
    fn, _ := FullNameFromURL(r.HTTPURLToRepo)
    if _, ok := repoMap[fn]; !ok {
        repoMap[fn] = r
        repoNames = append(repoNames, fn)
    }
}
  1. upstream (alpine/aports) is checked
  2. it is not a fork so just pass &repo to the add function which adds it to repoMap[alpine/aports] which will point to the reference (due to &repo) and repoNames
  3. origin (Leo/aports) is checked
  4. origin (Leo/aports) is a fork!, check for what it is forked from (alpine/aports)
  5. pass the fork origin to add, which does nothing because we already added repoMap[alpine/aports] and repoNames from the upstream remote
  6. pass &repo to the add function which adds it to repoMap[Leo/aports] and repoNames

both repoMap[alpine/aports] and repoMap[Leo/aports] now point to the same address, and the later one (Leo/aports), overwrote the earlier one (alpine/aports)!

Related Issue

Resolves #385 (closed)

How Has This Been Tested? Tested it against the usecase presented in #385 (closed)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation
  • Chore (Related to CI or Packaging to platforms)

Merge request reports