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

get_main_branch_config() reliance on length(tablename) fails with 0 row table #767

Closed
D3SL opened this issue Apr 28, 2024 · 2 comments · Fixed by #775
Closed

get_main_branch_config() reliance on length(tablename) fails with 0 row table #767

D3SL opened this issue Apr 28, 2024 · 2 comments · Fixed by #775

Comments

@D3SL
Copy link
Contributor

D3SL commented Apr 28, 2024

I just found this package and was very excited, unfortunately I haven't been able to get it to work yet. I've troubleshot a few of the stumbling blocks to getting fledge::bump_version() to work but this one seems to be impassable.

In bump-version.R get_main_branch_config() (line 98) relies on the test length(local) (line 103). Dataframes, tibbles, and data.tables all return their number of columns for length(), even if they are empty. This causes get_main_branch_config() to spuriously return init[init$level == "local", ]$value (line 104) instead of init[init$level == "global"]$value (line 108)

Below is a redacted sample of my exact data to demonstrate

# object 'config' as would be retrieved by gert::git_config(repo)
config<-structure(list(name = c("diff.astextplain.textconv", "filter.lfs.clean", 
"filter.lfs.smudge", "filter.lfs.process", "filter.lfs.required", 
"http.sslbackend", "http.sslcainfo", "core.autocrlf", "core.fscache", 
"core.symlinks", "pull.rebase", "credential.helper", "credential.https://dev.azure.com.usehttppath", 
"core.editor", "core.excludesfile", "gui.recentrepo", "user.email", 
"user.name", "init.defaultbranch", "core.repositoryformatversion", 
"core.filemode", "core.bare", "core.logallrefupdates", "core.symlinks", 
"core.ignorecase", "remote.origin.url", "remote.origin.fetch", 
"branch.main.remote", "branch.main.merge"), value = c("astextplain", 
"git-lfs clean -- %f", "git-lfs smudge -- %f", "git-lfs filter-process", 
"true", "openssl", "C:/Program Files/Git/mingw64/ssl/certs/ca-bundle.crt", 
"true", "true", "false", "false", "manager-core", "true", "\"C:\\Users\\Foo\\AppData\\Local\\Programs\\Microsoft VS Code\\Code.exe\" --wait", 
"C:/Users/Foo/.gitignore", "C:/foo/bar/baz/foobar", 
"foo@bar.baz", "Foo Bar", "main", "0", "false", 
"false", "true", "false", "true", "http://giteahere:3000/Foo/Bar.git", 
"+refs/heads/*:refs/remotes/origin/*", "origin", "refs/heads/main"
), level = c("system", "system", "system", "system", "system", 
"system", "system", "system", "system", "system", "system", "system", 
"system", "global", "global", "global", "global", "global", "global", 
"local", "local", "local", "local", "local", "local", "local", 
"local", "local", "local")), row.names = c(NA, 29L), class = c("tbl_df", 
"tbl", "data.frame"))

# contents of get_main_branch_config() configured to run interactively
init <- config[config$name == "init.defaultbranch", ]
print(init)

# A tibble: 1 × 3
#  name               value  level 
#  <chr>              <chr>  <chr> 
#1 init.defaultbranch main global

local <- init[init$level == "local", ] 
print(local)
# A tibble: 0 × 3
# ℹ 3 variables: name <chr>, value <chr>, level <chr>

if (length(local)) {  # this test fails because length(local)==3, _nrow(local)_==0
  print(local$value)    # the function prematurely ends, returning an empty table here
} else {
  global <- init[init$level == "global"] # this is missing a comma and would also break
  print(global$value)
}

If you want to stick with base R this is what I would suggest:

  get_main_branch_config <- function(repo) {
  #retrieve config of repo, filter down to init.defaultbranch values
  config <- gert::git_config(repo)
  init <- config[config$name == "init.defaultbranch", ]

  #return local default branch if it exists, otherwise default to global
  if("local" %in% init$level){
    return(init[init$level == "local",]$value)
  } else { 
    return(init[init$level == "global",]$value)
  }
}

You could also just get rid of this function entirely.

get_main_branch <- function(repo = getwd()) {
  remote <- "origin"            #hardcoded "origin" remote name 
  remote_list <- gert::git_remote_list(repo)
  if (remote %in% remote_list$name) {
    remote_main <- get_main_branch_remote(remote, repo)
    if (length(remote_main)) {
      return(remote_main)
    }
  } else{ 
    init<-gert::git_config(getwd())[config$name == "init.defaultbranch",]
    if("local" %in% init$level){
      return(init[init$level == "local",]$value)
    } else { 
      return(init[init$level == "global",]$value)
    }
  }
}
@maelle
Copy link
Member

maelle commented May 17, 2024

Thank you!! Would you like to make a PR with what comes after "If you want to stick with base R this is what I would suggest:"?

@D3SL
Copy link
Contributor Author

D3SL commented May 19, 2024

Done in #775 , I think. I've never actually made a pull request to a third party repo before so I'm not 100% sure I did that right.

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

Successfully merging a pull request may close this issue.

2 participants