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

Add an input option in r-cmd-check workflow to let users include additional install arguments #144

Closed
iantaylor-NOAA opened this issue Nov 6, 2024 · 15 comments · Fixed by #147
Assignees

Comments

@iantaylor-NOAA
Copy link
Collaborator

What is your question?

The new release of SS3 created some errors in github actions that (../ss3: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version GLIBCXX_3.4.32' not found (required by ../ss3)` @e-perl-NOAA seems to have solved by adding the missing library via the lines below in nmfs-ost/ss3-user-examples@7173d5e

        run: |
          sudo apt-get update
          sudo apt-get install -y libcurl4-openssl-dev
          sudo add-apt-repository ppa:ubuntu-toolchain-r/test
          sudo apt-get install --only-upgrade libstdc++6

I'm getting the same error in {r4ss} in call-r-cmd-check, but I'm assuming that those lines would have to occur after the matrix_prep step and therefore can't just be added prior to calling the {ghactions4r} workflow.

@Bai-Li-NOAA and @e-perl-NOAA, do you have any guidance on how to sort this out?

Additional context

No response

@Bai-Li-NOAA
Copy link
Collaborator

I see @e-perl-NOAA is working on updating the r-cmd-check.yml. I'll keep an eye out and am happy to help if needed!

@iantaylor-NOAA and @e-perl-NOAA I am also curious whether libcurl4-openssl-dev and libstdc++6 are specifically required for the new SS3 release, or if they’re generally needed across R packages. If it's only for SS3, maybe we could add an argument in use_r_cmd_check() (similar to depends_on_tmb argument) to allow users to specify additional tool installations as needed. If these libraries are required for R packages overall, We can add a conditional step for installation when runner.os == ubuntu-latest. Let me know what you think!

@e-perl-NOAA
Copy link
Contributor

Yes, I think that it is just an SS3 thing. Right now I'm just trying to get things working but after I do we can see if there is a way that we don't have to integrate it into the r_cmd_check() workflow directly.

@e-perl-NOAA
Copy link
Contributor

Okay so I have it working in my branch by adding those lines in the r_cmd_check workflow.
The following lines are r-related ubuntu lines that I've had to use for awhile to get r4ss working properly in our ss3-test-models repo

sudo apt-get install -y libcurl4-openssl-dev
sudo add-apt-repository ppa:ubuntu-toolchain-r/test

This line is ss3 ubuntu specific. There something with the ubuntu latest runner that despite the ss3 exe being built on it, the is a gcc library issue when running it that is only solved by adding this line.

sudo apt-get install --only-upgrade libstdc++6

@Bai-Li-NOAA
Copy link
Collaborator

Sounds great! @e-perl-NOAA Do you think we can create a pull request with your changes in the r_cmd_check workflow to get the SS3 repos passing GitHub Actions? In the meantime, we can keep this issue open, and I can set aside some time next week to add an input option that lets users include additional install arguments, following the example here. Does that work for you?

@e-perl-NOAA
Copy link
Contributor

Yes, that works! Thanks so much!

@Bai-Li-NOAA Bai-Li-NOAA added new_feature and removed question Further information is requested triage_needed This issue needs review labels Nov 7, 2024
@Bai-Li-NOAA Bai-Li-NOAA changed the title [Question]: how to add libraries? Add an input option in r-cmd-check workflow to let users include additional install arguments Nov 7, 2024
@chantelwetzel-noaa
Copy link

I am having a similar problem I think with the {nwfscDiag} r-cmd-check that install SS3 on linux to run tests. I have reviewed the changes in the r4ss pull request and it is not immediately clear to me how to apply those similar changes to my package. Additionally, it appears that the pkgdown action in r4ss is now failing once this pull request was merged. Since I don't fully understand the issue with the linux install, I am wondering if this is actually an issue with the linux version of SS3. Any thoughts on this and suggestion of how to revise the nwfscDiag actions?

@Bai-Li-NOAA
Copy link
Collaborator

@chantelwetzel-noaa, could you try adding the following code before - name: Set up R in r-cmd-check.yml? Let me know if this doesn't work for you!

- name: update linux libraries
  if: matrix.config.os == 'ubuntu-latest'
  run: |
    sudo apt-get update
    sudo apt-get install -y libcurl4-openssl-dev
    sudo add-apt-repository ppa:ubuntu-toolchain-r/test
    sudo apt-get install --only-upgrade libstdc++6

Also, thanks for reporting the pkgdown action failure. Since call-update_pkgdown.yml is a separate workflow, we'll need to make similar changes in in {ghactions4r}'s update-pkgdown.yml . I will work on adding an input optionto let users include additional install arguments and keep you posted on updating call-update_pkgdown.yml.

Regarding the Linux version issue with SS3, the actions are using the latest version of Ubuntu. I noticed that libcurl4 was installed on Ubuntu2204 but not on the latest Ubuntu 2404.

@chantelwetzel-noaa
Copy link

@Bai-Li-NOAA Thank you. This fixed the issues for nwfscDiag.

@chantelwetzel-noaa
Copy link

Actually, I was mistaken. I saw green actions at the top of the list, but those were not associated with the r-cmd-check action. I have added the above lines but the action is still failing with the same error:

Error: Error: R CMD check found ERRORs
Execution halted
── Test failures ───────────────────────────────────────────────── testthat ────

library(testthat)
library(nwfscDiag)

test_check("nwfscDiag")
./ss3: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version `GLIBCXX_3.4.32' not found (required by ./ss3)

@kellijohnson-NOAA
Copy link
Collaborator

I talked with @msupernaw about this because he is versed in linux and he suggested install g++ using

sudo apt-git install g++

@Bai-Li-NOAA
Copy link
Collaborator

@chantelwetzel-noaa, my mistake! You don’t need to include if: matrix.config.os == 'ubuntu-latest'. The update step was skipped because there isn’t a matrix.config.os configured in the setup.

@Bai-Li-NOAA
Copy link
Collaborator

Thanks, @kellijohnson-NOAA. The Ubuntu latest includes g++ version 4:13.2.0-7ubuntu1 and gcc version 4:13.2.0-7ubuntu1. There might be a compatibility issue between the installed gcc and the required libstdc++ by SS3.

@e-perl-NOAA
Copy link
Contributor

e-perl-NOAA commented Nov 13, 2024

Yeah I have no idea why, I built the linux version using the ubuntu-latest.... They update their runners so often, I can try building the latest release of ss3 from source again and see if maybe the issue that was in the initial build is resolved by now? That happened with the new macos-15 runner. There was an issue awhile back with it that I banged my head for so long trying to solve only to come back a couple weeks later to find it was resolved by updates that GitHub made to that runner.

@e-perl-NOAA
Copy link
Contributor

@chantelwetzel-noaa could you try calling the update-pkgdown workflow using:

uses: nmfs-fish-tools/ghactions4r/.github/workflows/update-pkgdown.yml@linux-pkgs-update-pkgdown

I added the lines of code that @Bai-Li-NOAA included above (which are the same as the ones that I added to the r-cmd-check.yml). Maybe adding it directly to the workflow will work better.

@k-doering-NOAA
Copy link
Collaborator

I think this has been addressed, but please reopen if I am wrong!

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

Successfully merging a pull request may close this issue.

6 participants