ropensci / rix

Reproducible Data Science environments for R with Nix
https://docs.ropensci.org/rix/
GNU General Public License v3.0
181 stars 15 forks source link

Imports can be empty #353

Open jrosell opened 3 weeks ago

jrosell commented 3 weeks ago

Related https://github.com/ropensci/rix/issues/352

jrosell commented 3 weeks ago

It seems that fetchgit is working in the new test. I used the same example as the issue https://github.com/ropensci/rix/issues/352

b-rodrigues commented 3 weeks ago

Could we test with another package than uwu? I cannot get the environment to build even when adding rustc and cargo to the nativeBuildInputs. The functionality seems to be fine, but I would like test if the environment is buildable.

b-rodrigues commented 3 weeks ago

Perhaps with poorman, it has no depends https://github.com/nathaneastwood/poorman

jrosell commented 3 weeks ago

I created a new small and dependency free package called {helloworld}. It's in path to CRAN too.

philipp-baumann commented 2 weeks ago

@jrosell thanks for the fix. @b-rodrigues @jrosell I think we need to do something with the intersect logic, I think we might have LinkingTo but no Depends and Imports, where we set to character(0) and might not work. I'm doing some more tests, I think the logic how to fix is good, but wanna be 100% sure before we release patch that we cover all edge cases in the application logic.

b-rodrigues commented 2 weeks ago

I think it's working as intended I just tried using the DESCRIPTION file from RcppEigen which only has a LinkingTo RCPP: https://github.com/RcppCore/RcppEigen/blob/master/DESCRIPTION

but this made me realize we should be testing get_imports() as well using some different DESCRIPTION files. @jrosell let me know if you wish to add these tests, if not I can take a stab at it or if you have some already @philipp-baumann we could add them as well

jrosell commented 2 weeks ago

Please, proceed as you prefer. I assume my PR is safer than the current main and you could merge and add tests afterwards, but it's up to you. I only made this PR for a current bug I was experiencing, but I guess more tests would be required in the long term.

philipp-baumann commented 2 weeks ago

Please, proceed as you prefer. I assume my PR is safer than the current main and you could merge and add tests afterwards, but it's up to you. I only made this PR for a current bug I was experiencing, but I guess more tests would be required in the long term.

library("rix")

pr_353 <- file.path("./pr_353")

rix(
  r_ver = "latest",
  r_pkgs = c("devtools", "styler", "lintr", "desc", "data.table"),
  git_pkgs = list(
    list(
      package_name = "rix",
      repo_url = "https://github.com/ropensci/rix",
      commit = "26825ecdc890b0b91842404656ea52be6b831f51"
    )
  ),
  project_path = pr_353,
  overwrite = TRUE
)

nix_build(project_path = pr_353)
library("devtools")
library("data.table")

pkgs_bh_links <- revdep(
  "BH",
  dependencies = c("LinkingTo"),
  recursive = FALSE,
  ignore = NULL,
  bioconductor = FALSE
)

# alpine linux
db <- tools::CRAN_package_db()
setDT(db)

db_noinput <-
  db[!is.na(`Reverse linking to`) & is.na(Depends) & is.na(Imports), ]

db_noinput$Package

pr_353_nloptr <- file.path(".", "pr_353", "nloptr")

rix(
  r_ver = "latest",
  git_pkgs = list(
    list(
      package_name = "https://github.com/astamm/nloptr",
      repo_url = "https://github.com/astamm/nloptr",
      commit = "6d4943aff5a47bd3b3914f86acaa5d6eeeccaa77"
    )
  ),
  project_path = pr_353_nloptr,
  overwrite = TRUE
)

nix_build(project_path = pr_353_nloptr)

pr_353_bh <- file.path(".", "pr_353", "bh")

rix(
  r_ver = "latest",
  git_pkgs = list(
    list(
      package_name = "BH",
      repo_url = "https://github.com/eddelbuettel/bh",
      commit = "3d8adc434004d0b219aa4f508dbe8c3ddc2d916b"
    )
  ),
  project_path = pr_353_bh,
  overwrite = TRUE
)
philipp-baumann commented 2 weeks ago

It's good to have a fix, so having a solid solution in a few days that covers all cases for a fix is more sustainable in my opinion, especially for proper versioning and particularly for a package where reproducibility is key, hence trust in solid dev.

jrosell commented 1 week ago

Please, proceed as you prefer. I assume my PR is safer than the current main and you could merge and add tests afterwards, but it's up to you. I only made this PR for a current bug I was experiencing, but I guess more tests would be required in the long term.

library("rix")

pr_353 <- file.path("./pr_353")

rix(
  r_ver = "latest",
  r_pkgs = c("devtools", "styler", "lintr", "desc", "data.table"),
  git_pkgs = list(
    list(
      package_name = "rix",
      repo_url = "https://github.com/ropensci/rix",
      commit = "26825ecdc890b0b91842404656ea52be6b831f51"
    )
  ),
  project_path = pr_353,
  overwrite = TRUE
)

nix_build(project_path = pr_353)
library("devtools")
library("data.table")

pkgs_bh_links <- revdep(
  "BH",
  dependencies = c("LinkingTo"),
  recursive = FALSE,
  ignore = NULL,
  bioconductor = FALSE
)

# alpine linux
db <- tools::CRAN_package_db()
setDT(db)

db_noinput <-
  db[!is.na(`Reverse linking to`) & is.na(Depends) & is.na(Imports), ]

db_noinput$Package

pr_353_nloptr <- file.path(".", "pr_353", "nloptr")

rix(
  r_ver = "latest",
  git_pkgs = list(
    list(
      package_name = "https://github.com/astamm/nloptr",
      repo_url = "https://github.com/astamm/nloptr",
      commit = "6d4943aff5a47bd3b3914f86acaa5d6eeeccaa77"
    )
  ),
  project_path = pr_353_nloptr,
  overwrite = TRUE
)

nix_build(project_path = pr_353_nloptr)

pr_353_bh <- file.path(".", "pr_353", "bh")

rix(
  r_ver = "latest",
  git_pkgs = list(
    list(
      package_name = "BH",
      repo_url = "https://github.com/eddelbuettel/bh",
      commit = "3d8adc434004d0b219aa4f508dbe8c3ddc2d916b"
    )
  ),
  project_path = pr_353_bh,
  overwrite = TRUE
)

What is the expected output of all these? I could add them to my PR.