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

Trigger Linux-Distributions jobs on PRs #2100

Closed
piponazo opened this issue Feb 15, 2022 · 3 comments · Fixed by #2101
Closed

Trigger Linux-Distributions jobs on PRs #2100

piponazo opened this issue Feb 15, 2022 · 3 comments · Fixed by #2101
Assignees
Labels
CI Issues related with CI jobs

Comments

@piponazo
Copy link
Collaborator

Is your feature request related to a problem?

I noticed that the "Nightly - Linux Distributions" builds is failing on the main branch after merging #2090. However, in that PR all the CI checks were passing.

Describe the solution you would like

It would be good to also trigger the "Linux Distributions" builds in PRs, so that we do not end up breaking something in mian without noticing in the PRs.

We would just need to change .github/workflows)/nightly_Linux_distributions.yml so that it runs on PRs.

Furthermore, we should fix the current issue on main. It is complaining about undefined references to std::filesystem:

[100%] Linking CXX executable ../bin/exiv2
CMakeFiles/exiv2.dir/actions.cpp.o: In function `(anonymous namespace)::metacopy(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int, bool)':
actions.cpp:(.text+0xa4e7): undefined reference to `std::filesystem::__cxx11::path::_M_split_cmpts()'
actions.cpp:(.text+0xa4f7): undefined reference to `std::filesystem::temp_directory_path[abi:cxx11]()'
actions.cpp:(.text+0xa50f): undefined reference to `std::filesystem::__cxx11::path::has_root_directory() cons
...

Something like this should work.

@piponazo piponazo added the CI Issues related with CI jobs label Feb 15, 2022
@piponazo piponazo self-assigned this Feb 15, 2022
@hassec
Copy link
Member

hassec commented Feb 15, 2022

Running the entire workflow on PRs might be a lot. (It's 48 builds)
But I do think it makes sense to have a build of one of the older distros in the PR pipeline 👍

@piponazo
Copy link
Collaborator Author

I agree that it might be intimidating to have so many CI stuff in PRs , but it is actually one of the CI pipelines that take less time:

image

It takes half of the time that the "Windows Matrix" pipeline last. And if the CI pipelines run in parallel we should not get any delay.

@hassec
Copy link
Member

hassec commented Feb 15, 2022

I didn't realize that pipeline was that quick. That's way quicker than many of the windows pipelines 😅
In that case 👍 from me

(but in that case let's rename it and have one workflow for "full linux" test)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Issues related with CI jobs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants