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

ApiOnly CI #3873

Closed
wants to merge 0 commits into from
Closed

ApiOnly CI #3873

wants to merge 0 commits into from

Conversation

kubo6472
Copy link

@kubo6472 kubo6472 commented Jun 8, 2023

Closes #3871

@kubo6472 kubo6472 closed this Jun 8, 2023
@kubo6472 kubo6472 force-pushed the master branch 2 times, most recently from d440cb6 to 545a593 Compare June 8, 2023 21:04
@kubo6472 kubo6472 reopened this Jun 8, 2023
@unixfox
Copy link
Member

unixfox commented Jun 8, 2023

In my opinion I think it's better to build the image inside the same workflow: https://github.com/iv-org/invidious/blob/master/.github/workflows/container-release.yml

@kubo6472
Copy link
Author

kubo6472 commented Jun 8, 2023

are you thinking another Dockerfile with this changed to include that apionly flag?

@kubo6472 kubo6472 marked this pull request as ready for review June 9, 2023 09:09
@kubo6472 kubo6472 requested a review from unixfox as a code owner June 9, 2023 09:09
@kubo6472
Copy link
Author

kubo6472 commented Jun 9, 2023

Hey, I did move the -Dapi_only from GH actions to the Dockerfiles. I suppose this can't be merged this way into master, right?

@SamantazFox
Copy link
Member

If you put -Dapi_only in the container file, all builds will be API-only !

@unixfox I think it should be preferable to use make in the container. What do you think?
That way we could pass build args from docker directly to make, without all that if logic.

@unixfox
Copy link
Member

unixfox commented Jun 9, 2023

If you put -Dapi_only in the container file, all builds will be API-only !

@unixfox I think it should be preferable to use make in the container. What do you think? That way we could pass build args from docker directly to make, without all that if logic.

Good idea, I have just been lazy to implement it ^^.

@unixfox
Copy link
Member

unixfox commented Jun 11, 2023

So the goal is to use make in the dockerfile, make the Makefile options configurable then in github actions for each building step use the correct Makefile options for each kind of build (normal build, api only build, quic build).

@kubo6472 kubo6472 requested a review from a team as a code owner July 1, 2023 12:19
@kubo6472
Copy link
Author

kubo6472 commented Jul 1, 2023

Tried using make to build docker images, probably wrong, may be on the right track. Please do take a look. Thanks

docker/Dockerfile Outdated Show resolved Hide resolved
@kubo6472
Copy link
Author

kubo6472 commented Jul 1, 2023

Makefile wasn't in the container that was running the build.

@kubo6472 kubo6472 changed the title ApiOnly CI (1/2) WIP ApiOnly CI Jul 1, 2023
@kubo6472 kubo6472 requested a review from SamantazFox July 1, 2023 21:52
@kubo6472
Copy link
Author

kubo6472 commented Jul 5, 2023

Please do take a look when you'll have time. Thanks!

@unixfox
Copy link
Member

unixfox commented Jul 5, 2023

Can you squash your commits?

docker/Dockerfile.arm64 Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@kubo6472
Copy link
Author

kubo6472 commented Jul 9, 2023

Due to the merges I am unable to squash further. I hope this isn't an issue. I've made the changes that were last requested.

@kubo6472 kubo6472 requested a review from SamantazFox July 9, 2023 12:20
@unixfox
Copy link
Member

unixfox commented Jul 18, 2023

You need to have just one commit remaining in https://github.com/iv-org/invidious/pull/3873/commits.
Check this guide: https://www.git-tower.com/learn/git/faq/git-squash

Or you can do it the ugly way, like I sometimes do. Copy somewhere else the changes that you made, remove the branch locally, create the same branch name and replace with the changes that you copied somewhere else.

@kubo6472 kubo6472 closed this Jul 18, 2023
@kubo6472 kubo6472 force-pushed the master branch 2 times, most recently from 68a837e to 69e2eac Compare July 18, 2023 22:36
@kubo6472 kubo6472 reopened this Jul 18, 2023
@kubo6472
Copy link
Author

best I could do, please let me know if it is okay now. for the future, I'm sure I'll prefer using branches like feature-apionly to not mess up the outdated master with the changes 😅

docker/Dockerfile.arm64 Outdated Show resolved Hide resolved
--static --warnings all \
--link-flags "-lxml2 -llzma"; \
fi
RUN make "RELEASE=${release}" "DISABLE_QUIC=${disable_quic}" "API_ONLY=${api_only}"
Copy link
Member

Choose a reason for hiding this comment

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

That one still remains ^^
Can you also rebase your changes?

@kubo6472
Copy link
Author

kubo6472 commented Oct 9, 2023

Should be good to go :) @SamantazFox

Copy link

github-actions bot commented Jan 8, 2024

This pull request has been automatically marked as stale and will be closed in 30 days because it has not had recent activity and is much likely abandoned or outdated. If you think this pull request is still relevant and applicable, you just have to post a comment and it will be unmarked.

@github-actions github-actions bot added the stale label Jan 8, 2024
@kubo6472
Copy link
Author

kubo6472 commented Jan 8, 2024

If I take the energy to update this again, will it be merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exempt-stale Exempt the issue from staling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Provide api_only docker image
4 participants