Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

Align name of release files for easier consumption #30

Closed
wants to merge 2 commits into from
Closed

Align name of release files for easier consumption #30

wants to merge 2 commits into from

Conversation

pjbgf
Copy link
Member

@pjbgf pjbgf commented Jul 8, 2022

This PR also split workflows in the attempt to decrease the time required to build/release.

Note that the workflow split between all-libs and libgit2-only is a step stone towards the decommissioning of Managed Transport. Once that takes place we no longer will require all-libs and the compilation times that comes with it.

Relates to fluxcd/source-controller#820 and fluxcd/image-automation-controller#404.

Copy link
Member

@aryan9600 aryan9600 left a comment

Choose a reason for hiding this comment

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

How about we move both the jobs that run the tests on Darwin to .github/workflows/test.yaml and then add jobs that run the tests on Linux to that file? (not necessarily in this PR)

.github/workflows/release.yaml Outdated Show resolved Hide resolved
.github/workflows/build.yaml Outdated Show resolved Hide resolved
.github/workflows/build.yaml Outdated Show resolved Hide resolved
.github/workflows/build.yaml Outdated Show resolved Hide resolved
.github/workflows/build.yaml Outdated Show resolved Hide resolved
.github/workflows/release.yaml Outdated Show resolved Hide resolved
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
@pjbgf pjbgf requested a review from aryan9600 July 11, 2022 23:35

libtool -static -o ./libgit2-darwin-libgit2-all/lib/libcrypto.a \
${GITHUB_WORKSPACE}/build/libgit2-darwin-amd64/lib/libcrypto.a \
LIBGIT2_SED="s;-L/Applications/Xcode_.* ;;g"
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be?

Suggested change
LIBGIT2_SED="s;-L/Applications/Xcode_.* ;;g"
LIBGIT2_SED="s;-L/Applications/Xcode.* ;;g"

Also, we should be doing this when we are building libgit2 only as well? This is what libgit2.pc looks like on my mac (running macOS 12) when I build libgit2 only:

prefix="/Users/sanskarjaiswal/Development/fluxcd/golang-with-libgit2/build/libgit2"
libdir="/Users/sanskarjaiswal/Development/fluxcd/golang-with-libgit2/build/libgit2/lib"
includedir="/Users/sanskarjaiswal/Development/fluxcd/golang-with-libgit2/build/libgit2/include"
                                                                                                                                   
Name: libgit2
Description: The git library, take 2
Version: 1.3.1
Libs: -L${libdir} -lgit2
Libs.private: -L/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.1.sdk/usr/lib -liconv
Cflags: -I${includedir}

Copy link
Member Author

Choose a reason for hiding this comment

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

This was extracted from the script we currently run at both IAC and SC and is based on the current runner's OS:
https://github.com/fluxcd/source-controller/blob/main/hack/install-libraries.sh#L59

I updated it so it will continue to work when we upgrade the runner OS to macOS 12.

Copy link
Member Author

Choose a reason for hiding this comment

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

Leaving this comment unresolved, in case doubts arise on this in the future.

.github/workflows/test.yaml Outdated Show resolved Hide resolved
.github/workflows/test.yaml Outdated Show resolved Hide resolved
@pjbgf pjbgf requested a review from aryan9600 July 12, 2022 07:22
Copy link
Member

@aryan9600 aryan9600 left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀 🙇

@pjbgf pjbgf requested a review from hiddeco July 12, 2022 09:46
@pjbgf
Copy link
Member Author

pjbgf commented Jul 12, 2022

Build seems stuck, do will close and reopen the PR as nothing else seem to get build back.

@pjbgf pjbgf closed this Jul 12, 2022
@pjbgf pjbgf reopened this Jul 12, 2022
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
@pjbgf pjbgf changed the base branch from main to nothread July 12, 2022 15:24
@pjbgf pjbgf changed the base branch from nothread to main July 12, 2022 15:24
@pjbgf pjbgf closed this Jul 12, 2022
@pjbgf
Copy link
Member Author

pjbgf commented Jul 13, 2022

Closed in favour of #32 due to the issues with GitHub checks.

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

Successfully merging this pull request may close these issues.

3 participants