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

enhanced subdir support for gitlab #182

Merged
merged 3 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Package: staged.dependencies
Type: Package
Title: Install R packages from Particular Git Branches
Version: 0.3.0
Version: 0.3.0.9001
Authors@R: c(
person("Adrian", "Waddell", email = "[email protected]", role = c("aut", "cre")),
person("Maximilian", "Mordig", email = "[email protected]", role = "aut"),
Expand Down
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# staged.dependecies 0.3.0.9001

* Enhanced support of subdirectories for gitlab.

# staged.dependecies 0.3.0

* Support subdirectories in configuration.
Expand Down
11 changes: 6 additions & 5 deletions R/caching.R
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ get_hashed_repo_to_dir_mapping <- function(local_repos) {
} else {
stopifnot(
is.data.frame(local_repos),
setequal(colnames(local_repos), c("repo", "host", "directory"))
setequal(colnames(local_repos), c("repo", "host", "subdir", "directory"))
)
stats::setNames(local_repos$directory, hash_repo_and_host(local_repos))
}
Expand Down Expand Up @@ -284,20 +284,21 @@ rec_checkout_internal_deps <- function(repos_to_process, ref,
)
}

repo_info$subdir <- parse_git_ref(repo_and_host$repo)$subdir
repo_info$subdir <- repo_and_host$subdir
repo_info$path <- fs::path_norm(fs::path_join(c(repo_info$dir, repo_info$subdir)))

hashed_new_repos <- c()
if (repo_info$accessible) {
if (direction %in% c("upstream", "all")) {
hashed_upstream_repos <- lapply(get_yaml_deps_info(fs::path_join(c(repo_info$dir, repo_info$subdir)))$upstream_repos, hash_repo_and_host)
hashed_upstream_repos <- lapply(get_yaml_deps_info(repo_info$path)$upstream_repos, hash_repo_and_host)
hashed_new_repos <- c(hashed_new_repos, hashed_upstream_repos)
}
if (direction %in% c("downstream", "all")) {
hashed_downstream_repos <- lapply(get_yaml_deps_info(fs::path_join(c(repo_info$dir, repo_info$subdir)))$downstream_repos, hash_repo_and_host)
hashed_downstream_repos <- lapply(get_yaml_deps_info(repo_info$path)$downstream_repos, hash_repo_and_host)
hashed_new_repos <- c(hashed_new_repos, hashed_downstream_repos)
}
}
hashed_processed_repos[[hashed_repo_and_host]] <- fs::path_join(c(repo_info$dir, repo_info$subdir))
hashed_processed_repos[[hashed_repo_and_host]] <- repo_info$path
hashed_repos_accessible[[hashed_repo_and_host]] <- repo_info$accessible
hashed_repos_refs[[hashed_repo_and_host]] <- repo_info$ref
hashed_repos_shas[[hashed_repo_and_host]] <- repo_info$sha
Expand Down
5 changes: 3 additions & 2 deletions R/dependencies_helper.R
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ add_project_to_local_repos <- function(project, local_repos) {
data.frame(
repo = repo_deps_info$current_repo$repo,
host = repo_deps_info$current_repo$host,
subdir = ".",
directory = normalize_path(project), stringsAsFactors = FALSE
)
)
Expand Down Expand Up @@ -359,7 +360,7 @@ yaml_from_dep_table <- function(dep_table) {
}


# take string x@y and split it into list(repo=x, host=y)
# take string x@y and split it into list(repo=x, host=y, subdir=z)
# error if multiple "@'s" in string, if no @'s then take
# host = "https://github.com"
parse_remote_project <- function(project) {
Expand All @@ -377,7 +378,7 @@ parse_remote_project <- function(project) {
if (any(nchar(trimws(split_string)) == 0)) {
stop(error_message)
}
return(list(repo = split_string[1], host = split_string[2]))
return(list(repo = split_string[1], host = split_string[2], subdir = "."))
}

# filter packages according to some criteria
Expand Down
10 changes: 1 addition & 9 deletions R/git_tools.R
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,7 @@ get_repo_url <- function(repo, host) {
is_non_empty_char(repo),
is_non_empty_char(host)
)
file.path(host, paste0(do.call(file.path, parse_git_ref(repo)[1:2]), ".git"))
}

parse_git_ref <- function(ref) {
list(
username = strsplit(ref, "/")[[1]][1],
repo = strsplit(ref, "/")[[1]][2],
subdir = paste0(strsplit(ref, "/")[[1]][-(1:2)], collapse = "/")
)
file.path(host, paste0(repo, ".git"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's so much cleaner 😌

}

# gets the currently checked out branch
Expand Down
9 changes: 7 additions & 2 deletions R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ hash_repo_and_host <- function(repo_and_host) {
if (length(repo_and_host) == 0) {
c()
} else {
paste0(repo_and_host$repo, " @ ", repo_and_host$host)
paste0(repo_and_host$repo, " @ ", repo_and_host$host, " @ ", repo_and_host$subdir)
}
}

Expand All @@ -55,7 +55,8 @@ unhash_repo_and_host <- function(hashed_repo_and_host) {
repo_and_host <- strsplit(hashed_repo_and_host, " @ ", fixed = TRUE)
list(
repo = extract_str_field(repo_and_host, 1),
host = extract_str_field(repo_and_host, 2)
host = extract_str_field(repo_and_host, 2),
subdir = extract_str_field(repo_and_host, 3)
)
}

Expand Down Expand Up @@ -196,6 +197,10 @@ get_yaml_deps_info <- function(repo_dir) {
if (file.exists(yaml_file)) {
content <- yaml::read_yaml(yaml_file)
validate_staged_deps_yaml(content, file_name = yaml_file)
# fill in optional subdir field if missing
if (is.null(content$current_repo$subdir)) content$current_repo$subdir <- "."
for (i in seq_along(content$upstream_repos)) if (is.null(content$upstream_repos[[i]]$subdir)) content$upstream_repos[[i]]$subdir <- "."
for (i in seq_along(content$downstream_repos)) if (is.null(content$downstream_repos[[i]]$subdir)) content$downstream_repos[[i]]$subdir <- "."
content
} else {
list(
Expand Down
9 changes: 5 additions & 4 deletions tests/testthat/test-caching.R
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ test_that("rec_checkout_internal_deps works (with mocking checkout)", {
# check error if fallback_branch argument is incorrect
expect_error(
capture.output(rec_checkout_internal_deps(
list(list(repo = "openpharma/stageddeps.food", host = "https://github.com")),
list(list(repo = "openpharma/stageddeps.food", host = "https://github.com", subdir = ".")),
"unittest_branch1",
direction = c("upstream"), local_repos = NULL, fallback_branch = "not_exist", verbose = 0
)),
regexp = "Available refs .* must include at least one of 'unittest_branch1, not_exist'"
)

output <- capture.output(res <- rec_checkout_internal_deps(
list(list(repo = "openpharma/stageddeps.food", host = "https://github.com")),
list(list(repo = "openpharma/stageddeps.food", host = "https://github.com", subdir = ".")),
"fix1@main",
direction = c("upstream"), local_repos = NULL, verbose = 0
))
Expand Down Expand Up @@ -132,7 +132,7 @@ test_that("rec_checkout_internal_deps works for inaccessible repos (with mocking
})

res <- rec_checkout_internal_deps(
list(list(repo = "openpharma/stageddeps.food", host = "https://github.com")),
list(list(repo = "openpharma/stageddeps.food", host = "https://github.com", subdir = ".")),
"main",
local_repos = NULL, verbose = 0, direction = "all"
)
Expand All @@ -152,11 +152,12 @@ test_that("get_hashed_repo_to_dir_mapping works", {
data.frame(
repo = "openpharma/stageddeps.food",
host = "https://github.com",
subdir = ".",
directory = "dummy_dir",
stringsAsFactors = FALSE
)
),
c(`openpharma/stageddeps.food @ https://github.com` = "dummy_dir")
c(`openpharma/stageddeps.food @ https://github.com @ .` = "dummy_dir")
)
})

Expand Down
3 changes: 2 additions & 1 deletion tests/testthat/test-dependencies.R
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ local_pkgs <- c("stageddeps.elecinfra", "stageddeps.electricity", "stageddeps.fo
mock_rec_checkout_internal_deps <- function(source_dir) {
function(repos_to_process, ...) {
cat(paste0("Mocking rec_checkout_internal_deps", "\n"))
expect_equal(repos_to_process, list(list(repo = "openpharma/stageddeps.food", host = "https://github.com")))
expect_equal(repos_to_process, list(list(repo = "openpharma/stageddeps.food", host = "https://github.com", subdir = ".")))

# stageddeps.food is local
internal_deps <- data.frame(
Expand Down Expand Up @@ -105,6 +105,7 @@ test_that("dependency_table wih local_pkgs works", {
local_repos <- data.frame(
repo = paste0("openpharma/", local_pkgs),
host = rep("https://github.com", 6),
subdir = ".",
directory = file.path(copied_ecosystem, local_pkgs),
stringsAsFactors = FALSE
)
Expand Down
9 changes: 5 additions & 4 deletions tests/testthat/test-dependencies_helper.R
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ test_that("add_project_to_local_repos works", {
expect_equal(
add_project_to_local_repos(project, local_repos = NULL),
data.frame(
repo = "openpharma/stageddeps.food", host = "https://github.com",
repo = "openpharma/stageddeps.food", host = "https://github.com", subdir = ".",
directory = normalize_path(project),
stringsAsFactors = FALSE
)
Expand All @@ -93,13 +93,14 @@ test_that("add_project_to_local_repos works", {
add_project_to_local_repos(
project,
local_repos = data.frame(
repo = "openpharma/stageddeps.electricity", host = "https://github.com",
repo = "openpharma/stageddeps.electricity", host = "https://github.com", subdir = ".",
directory = normalize_path(file.path(TESTS_GIT_REPOS, "stageddeps.electricity")), stringsAsFactors = FALSE
)
),
data.frame(
repo = c("openpharma/stageddeps.electricity", "openpharma/stageddeps.food"),
host = c("https://github.com", "https://github.com"),
subdir = ".",
directory = c(
normalize_path(file.path(TESTS_GIT_REPOS, "stageddeps.electricity")),
normalize_path(project)
Expand Down Expand Up @@ -140,14 +141,14 @@ test_that("parse_remote_project works", {
# repo@host
expect_equal(
parse_remote_project("x@y"),
list(repo = "x", host = "y")
list(repo = "x", host = "y", subdir = ".")
)


# missing host uses default
expect_equal(
parse_remote_project("x"),
list(repo = "x", host = "https://github.com")
list(repo = "x", host = "https://github.com", subdir = ".")
)

# more than 1 @ throws error
Expand Down
16 changes: 8 additions & 8 deletions tests/testthat/test-utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,25 @@ test_that("hash_repo_and_host works", {
expect_null(hash_repo_and_host(list()))
# when lists contains one element
expect_equal(
hash_repo_and_host(list(repo = "repo1", host = "host1")),
"repo1 @ host1"
hash_repo_and_host(list(repo = "repo1", host = "host1", subdir = "test1")),
"repo1 @ host1 @ test1"
)
# when lists contain two elements
expect_equal(
hash_repo_and_host(list(repo = c("repo1", "repo2"), host = c("host1", "host2"))),
c("repo1 @ host1", "repo2 @ host2")
hash_repo_and_host(list(repo = c("repo1", "repo2"), host = c("host1", "host2"), subdir = c("test1", "test2"))),
c("repo1 @ host1 @ test1", "repo2 @ host2 @ test2")
)
})

test_that("unhash_repo_and_host works", {
# when empty
expect_equal(unhash_repo_and_host(character(0)), list(repo = character(0), host = character(0)))
expect_equal(unhash_repo_and_host(character(0)), list(repo = character(0), host = character(0), subdir = character(0)))
# when of size 1
expect_equal(unhash_repo_and_host("repo1 @ host1"), list(repo = "repo1", host = "host1"))
expect_equal(unhash_repo_and_host("repo1 @ host1 @ test1"), list(repo = "repo1", host = "host1", subdir = "test1"))
# when of size 2
expect_equal(
unhash_repo_and_host(c("repo1 @ host1", "repo2 @ host2")),
list(repo = c("repo1", "repo2"), host = c("host1", "host2"))
unhash_repo_and_host(c("repo1 @ host1 @ test1", "repo2 @ host2 @ test2")),
list(repo = c("repo1", "repo2"), host = c("host1", "host2"), subdir = c("test1", "test2"))
)
})

Expand Down