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.

Sign up for GitHub

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

Jump to bottom

pkg: unify git checkout #5138

Merged
merged 13 commits into from
Mar 23, 2016
Merged

pkg: unify git checkout #5138

merged 13 commits into from
Mar 23, 2016

Conversation

kaspar030
Copy link
Contributor

This PR introduces a shared makefile for all the git checkout/patch/clean stuff.

Packages are now checked out into $(BINDIR)/pkg/<pkg_name>, solving potential parallel-build-for-multiple-platform problems.
Patches went into patches subdirectories.

Fixes #5130 #4962 #4511.

@kaspar030 kaspar030 added Area: build system Area: Build system Area: pkg Area: External package ports Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 22, 2016
@kaspar030 kaspar030 added this to the Release 2016.04 milestone Mar 22, 2016
@kaspar030
Copy link
Contributor Author

Why are all pkg tests skipped?

miri64
miri64 reviewed Mar 22, 2016
View reviewed changes
pkg/pkg.mk

$(PKG_BUILDDIR)/.downloaded:
mkdir -p $(PKG_BUILDDIR)
$(GITCACHE) clone "$(PKG_URL)" "$(PKG_VERSION)" "$(PKG_BUILDDIR)"
Copy link
Member

Choose a reason for hiding this comment

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

Not every package must be a git repository. I would prefer to call the file pkg.git.mk or something like that instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I called it "pkg.mk" because we will extend it, and then either some logic will decide wether a pkg is "git", or the targets can be called "git-checkout", or whatever.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@authmillenon you fine with this?

Copy link
Member

Choose a reason for hiding this comment

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

Yes.

@miri64
Copy link
Member

miri64 commented Mar 22, 2016

Why are all pkg tests skipped?

Are there applications including them? Maybe the is_updated() in compile_test.py specifically or the info-files Make target in general have a problem with the fact the files do not exist pre-build...

@kaspar030
Copy link
Contributor Author

Why are all pkg tests skipped?

Are there applications including them? Maybe the is_updated() in compile_test.py specifically or the info-files Make target in general have a problem with the fact the files do not exist pre-build...

All but two packages get included by applications, so I guess is_updated() is to blame.

@kaspar030 kaspar030 force-pushed the pkg_unify_git_checkout branch from 6215da2 to 407a93a Compare March 23, 2016 13:38
@kaspar030
Copy link
Contributor Author

@miri64
Copy link
Member

miri64 commented Mar 23, 2016

Does not fix #4511 btw. Relic still spams the output of the unittests full with cmake stuff.

@miri64
Copy link
Member

miri64 commented Mar 23, 2016

and there seems to be a problem with the ccn-lite package.

@kaspar030
Copy link
Contributor Author

Does not fix #4511 btw. Relic still spams the output of the unittests full with cmake stuff.

The issue description is "This makes it impossible to build for multiple platforms without clean inbetween.", which is fixed.

@OlegHahm
Copy link
Member

Relic still spams the output of the unittests full with cmake stuff.

Which is IMO acceptable. As long if it doesn't break anything in our build system it is IMO always preferable to do as little changes as possible to the upstream repository.

@OlegHahm OlegHahm added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Mar 23, 2016
@miri64
Copy link
Member

miri64 commented Mar 23, 2016

Okay from that perspective you are right.

@kaspar030 kaspar030 force-pushed the pkg_unify_git_checkout branch from 7363377 to 0cacc8b Compare March 23, 2016 14:42
@kaspar030
Copy link
Contributor Author

Murdock was happy, I squashed.

@kaspar030 kaspar030 removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Mar 23, 2016
@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 23, 2016
@kaspar030
Copy link
Contributor Author

Murdock happy. @authmillenon care to ACK? :)

@miri64
Copy link
Member

miri64 commented Mar 23, 2016

Yes ACK. Will provide a fix for allow non-git repos in the future. I have an idea how to make that very simple.

@miri64 miri64 merged commit f0eaed7 into RIOT-OS:master Mar 23, 2016
@miri64 miri64 mentioned this pull request Mar 23, 2016
Merged
@OlegHahm OlegHahm mentioned this pull request Mar 23, 2016
Closed
@kaspar030 kaspar030 mentioned this pull request Apr 19, 2016
Closed
@miri64 miri64 mentioned this pull request Apr 20, 2016
Closed
@kaspar030 kaspar030 deleted the pkg_unify_git_checkout branch February 7, 2017 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@OlegHahm OlegHahm

Labels
Area: build system Area: Build system Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Milestone
Release 2016.04
Development

Successfully merging this pull request may close these issues.
3 participants
@kaspar030 @miri64 @OlegHahm