Skip to content

Conversation

@plaes
Copy link
Contributor

@plaes plaes commented Oct 23, 2024

Some Fedora-related housekeeping due to upcoming Fedora 41 release:
Firstly, include cmake submodule using full repository url, which allows running Github actions without hardcoded relative repository locations.
And second and somewhat more controversial change would be the inclusion of rawhide in the Fedora build matrix. If there are objections, this could be dropped.

@metsma
Copy link
Contributor

metsma commented Oct 23, 2024

Firstly, include cmake submodule using full repository url

This complicates development in forks and we haven't seen any issues with this for years. Why qt-common submodule does not give errors?

@plaes
Copy link
Contributor Author

plaes commented Oct 23, 2024

Firstly, include cmake submodule using full repository url

This complicates development in forks and we haven't seen any issues with this for years. Why qt-common submodule does not give errors?

I don't see any qt-common submodule in master branch:
https://github.com/open-eid/libdigidocpp/blob/fb38ed4bb0b80f74da7451971ab2d1de34b6166c/.gitmodules

@metsma
Copy link
Contributor

metsma commented Oct 23, 2024

Firstly, include cmake submodule using full repository url

This complicates development in forks and we haven't seen any issues with this for years. Why qt-common submodule does not give errors?

I don't see any qt-common submodule in master branch: https://github.com/open-eid/libdigidocpp/blob/fb38ed4bb0b80f74da7451971ab2d1de34b6166c/.gitmodules

I am sorry, I thought that this was the DigiDoc4 repository

@plaes
Copy link
Contributor Author

plaes commented Oct 23, 2024

Anyway, as per "this complicates development in forks" is actually orthogonal to my experience as you would need to have set up cmake repositories in following places:

  • ../cmake in order to build repository on your machine
  • cmake repository under your account/organization to run github actions in your fork

@metsma
Copy link
Contributor

metsma commented Oct 23, 2024

git clone --recursive handles this well?
The problem arises when you want make changes in submodule then it is nearly impossible to test it out in own fork.
I agree that submodules have their problems and I have plan to get rid of cmake submodule completely.

@plaes
Copy link
Contributor Author

plaes commented Oct 23, 2024

git clone --recursive handles this well? The problem arises when you want make changes in submodule then it is nearly impossible to test it out in own fork. I agree that submodules have their problems and I have plan to get rid of cmake submodule completely.

Well, problem with current setup has two "issues" which require two separate workarounds:

  1. If I want to build libdigidocpp from git checkout (on my machine), I would also need to check out the open-eid/cmake repository and place it next to libdigidocpp repository so ../cmake path is resolved.
  2. I want to push a commit to a forked repository, github actions fail because it expects that cmake repository/fork is also present in my account. If it doesn't, github actions fail under my account:
Fetching submodules
  /opt/homebrew/bin/git submodule sync --recursive
  /opt/homebrew/bin/git -c protocol.version=2 submodule update --init --force --depth=1 --recursive
  Submodule 'cmake' (https://github.com/plaes/cmake) registered for path 'cmake'
  Cloning into '/Users/runner/work/libdigidocpp/libdigidocpp/cmake'...
  remote: Repository not found.
  Error: fatal: repository 'https://github.com/plaes/cmake/' not found
  Error: fatal: clone of 'https://github.com/plaes/cmake' into submodule path '/Users/runner/work/libdigidocpp/libdigidocpp/cmake' failed
  Failed to clone 'cmake'. Retry scheduled
  Cloning into '/Users/runner/work/libdigidocpp/libdigidocpp/cmake'...
  remote: Repository not found.
  Error: fatal: repository 'https://github.com/plaes/cmake/' not found
  Error: fatal: clone of 'https://github.com/plaes/cmake' into submodule path '/Users/runner/work/libdigidocpp/libdigidocpp/cmake' failed
  Failed to clone 'cmake' a second time, aborting
  Error: The process '/opt/homebrew/bin/git' failed with exit code 1

git clone --recursive works fine when checking out the repository from [email protected]:open-eid/libdigidocpp.git, because ../cmake points here: [email protected]:open-eid/cmake.git.
But everything falls apart when I would like to clone repository from my own account, as it expects to find the cmake repository relative to the account.

Anyway, I can drop this patch as I only occasionally check out the Fedora things and judging by the git log whole development process seems to be one man show anyway...

@metsma
Copy link
Contributor

metsma commented Oct 23, 2024

I have some Fedora changes already in #633 can I update my PR with your changes or do I rebase it on it?

@plaes
Copy link
Contributor Author

plaes commented Oct 23, 2024

I have some Fedora changes already in #633 can I update my PR with your changes or do I rebase it on it?

Probably easier to just copy them - most of the stuff I did there was reshuffling the deps into alphabetic order and then move required dependencies to variable in a similar way as Ubuntu has them.

@metsma
Copy link
Contributor

metsma commented Nov 8, 2024

Can you please rebase to latest master

@plaes plaes force-pushed the fix-cmake-submodule branch from 69dd5c8 to ca49977 Compare November 8, 2024 12:42
plaes added 2 commits November 8, 2024 14:58
Fedora 39 drops is EOL from 12 Nov 2024.

Signed-off-by: Priit Laes <[email protected]>
Additionally splits out list of required Fedora dependencies
into separate variable in github actions.

Fixes open-eid#637

Signed-off-by: Priit Laes <[email protected]>
@plaes plaes force-pushed the fix-cmake-submodule branch 3 times, most recently from b14f3db to 220167f Compare November 8, 2024 13:09
Include Rawhide in the build matrix as well to spot upcoming issues,
first one would be dropping support for OpenJDK 17 in favor of
Adoptium managed Eclipse Temurin releases via separate repsitory:
https://fedoraproject.org/wiki/Changes/ThirdPartyLegacyJdks

Ideally this build should be gated as `allow-fail`, but
unfortunately github actions does not support it yet.

Signed-off-by: Priit Laes <[email protected]>
@plaes plaes force-pushed the fix-cmake-submodule branch from 220167f to c561bdb Compare November 8, 2024 13:10
plaes added 2 commits November 8, 2024 22:11
Java 17 was already deprecated since Fedora 39, and
is already dropped in upcoming Fedora 42 (current Rawhide).

Signed-off-by: Priit Laes <[email protected]>
Apparently we need to use minizip-ng-compat library for MiniZip support

Signed-off-by: Priit Laes <[email protected]>
@kristelmerilain kristelmerilain merged commit 47168f7 into open-eid:master Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants