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

Makefile: user gomodcache from host in cross-builds #957

Merged
merged 1 commit into from Dec 14, 2022

Conversation

marquiz
Copy link
Contributor

@marquiz marquiz commented Dec 13, 2022

Speeds up the builds dramatically by eliminating a lot of go mod download time.

@codecov-commenter
Copy link

codecov-commenter commented Dec 13, 2022

Codecov Report

Merging #957 (6a27748) into master (443ced9) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #957   +/-   ##
=======================================
  Coverage   34.06%   34.06%           
=======================================
  Files          61       61           
  Lines        9054     9054           
=======================================
  Hits         3084     3084           
  Misses       5687     5687           
  Partials      283      283           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Makefile Outdated
echo "Docker cross-building $$distro packages..."; \
mkdir -p $(PACKAGES_DIR)/$$distro && \
rm -fr $$builddir && mkdir -p $$builddir/{input,build} && \
cp cri-resource-manager-$(TAR_VERSION).tar$(GZEXT) $$builddir/input && \
cp packaging/rpm/cri-resource-manager.spec $$builddir/input && \
$(DOCKER) run --rm -ti $(DOCKER_OPTIONS) --user $(shell echo $$USER) \
$(DOCKER) run --rm -ti $(DOCKER_OPTIONS) --user $$user \
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity: is setting $user then dereferencing it vs. using $USER a workaround/fix for running in non-login shells ? IOW, what does it fix ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't actually fix anything, although I thought it might be more reliable than relying on env variables. Maybe $USER would be enough

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to $USER to keep the changes minimal (and simplify a bit)

Copy link
Contributor

@klihub klihub left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks ! This is really good stuff... should speed up cross-builds in everyday testing greatly. I just have a question to understand part of the changes.

Speeds up the builds dramatically by eliminating a lot of go mod
download time.
Copy link
Contributor

@askervin askervin left a comment

Choose a reason for hiding this comment

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

Nice! Saved 20 % of cross-package build time on my desktop!

Makefile Show resolved Hide resolved
Makefile Show resolved Hide resolved
@klihub klihub merged commit 2784a1c into intel:master Dec 14, 2022
@marquiz marquiz deleted the devel/cross-build-cache branch December 15, 2022 11:47
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.

None yet

5 participants