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

tinydtls: fix for build system fix #6053

Merged
merged 1 commit into from
Nov 4, 2016
Merged

tinydtls: fix for build system fix #6053

merged 1 commit into from
Nov 4, 2016

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Nov 3, 2016

Do not merge until rfuentess/TinyDTLS#1 was resolved and the PKG_VERSION of tinydtls updated accordingly in this PR!

@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking Area: pkg Area: External package ports labels Nov 3, 2016
@miri64 miri64 added this to the Release 2016.10 milestone Nov 3, 2016
@miri64
Copy link
Member Author

miri64 commented Nov 3, 2016

Fixes #6036 (I hope)

OlegHahm
OlegHahm reviewed Nov 3, 2016
View reviewed changes
examples/dtls-echo/Makefile
@@ -49,6 +49,8 @@ CFLAGS += -DDEVELHELP
# NOTE: Add the package for TinyDTLS
USEPKG += tinydtls

USEMODULE += tinydtls
Copy link
Member

Choose a reason for hiding this comment

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

A module and a package with the same name?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, as far as I understood #5891 the idea now:

  • USEPKG just is responsible for getting the package, while
  • USEMODULE is responsible for building it as a module.

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 is not properly cleaned up in master, so we can't use it as an example ;-))

Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed for this package, but not for other packages (e.g., ccn-lite) then?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because this package already has a pkg/$(PKG_NAME)/Makefile.dep.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

(then the pkg's Makefile.dep needs to be changed to check for tinydtls in USEPKG.

Copy link
Member Author

@miri64 miri64 Nov 4, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

abroe los ojos https://github.com/RIOT-OS/RIOT/pull/6053/files#diff-beb0aa38ffeadc49704a44f1aeee9b41R1

Copy link
Member

Choose a reason for hiding this comment

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

Supongo que es muy útil de abrir los ojos para postear un link. 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed :P

@rfuentess rfuentess mentioned this pull request Nov 3, 2016
Closed
@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 3, 2016
@miri64
Copy link
Member Author

miri64 commented Nov 3, 2016

Wait the branch you referenced doesn't provide the patch...

@miri64 miri64 removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 3, 2016
@miri64
Copy link
Member Author

miri64 commented Nov 3, 2016

Updated board blacklist according to @rfuentess reporting (and also adapted memory unsufficient list accordingly ;-))

@miri64
Copy link
Member Author

miri64 commented Nov 4, 2016

Of course I forgot to add the Makefile.dep I was talking about in #6053 (comment). @rfuentess this fixes the issue we talked about on IRC.

@miri64 miri64 mentioned this pull request Nov 4, 2016
Merged
@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 Nov 4, 2016
@miri64
Copy link
Member Author

miri64 commented Nov 4, 2016

Let's try building this a few times to make sure #6036 is really fixed.

@miri64
Copy link
Member Author

miri64 commented Nov 4, 2016

success no 2

@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 Nov 4, 2016
@miri64
Copy link
Member Author

miri64 commented Nov 4, 2016

success no 3

@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 Nov 4, 2016
@rfuentess
Copy link
Contributor

Meanwhile murdock is running, I'm testing directly on my local copy. Right now, 17 successful testing of make distclean; make buildtest

@miri64
Copy link
Member Author

miri64 commented Nov 4, 2016

I will do five runs of Murdock (minus the one after rebase).

@rfuentess
Copy link
Contributor

As long Murdock is OK I think the changes are good, so, ACK !

@miri64
Copy link
Member Author

miri64 commented Nov 4, 2016

success no 4

@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 Nov 4, 2016
@miri64
Copy link
Member Author

miri64 commented Nov 4, 2016

success no 5 \o/ rebasing now.

@miri64 miri64 force-pushed the tinydtls/fix/buildsystem branch from bb6f76b to d50d5d6 Compare November 4, 2016 15:20
@miri64
Copy link
Member Author

miri64 commented Nov 4, 2016

Done

@miri64 miri64 force-pushed the tinydtls/fix/buildsystem branch from d50d5d6 to 21c4cb4 Compare November 4, 2016 15:21
OlegHahm
OlegHahm approved these changes Nov 4, 2016
View reviewed changes
Copy link
Member

@OlegHahm OlegHahm left a comment

Choose a reason for hiding this comment

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

ACK on the behalf of @rfuentess

@miri64 miri64 mentioned this pull request Nov 4, 2016
Merged
@miri64
Copy link
Member Author

miri64 commented Nov 4, 2016

Backport provided in #6061.

@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 Nov 4, 2016
@miri64
Copy link
Member Author

miri64 commented Nov 4, 2016

Murdock fail weird but unrelated (for once ^^)

@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 Nov 4, 2016
@miri64
Copy link
Member Author

miri64 commented Nov 4, 2016 •
edited
Loading

(just FYI, it was caused by #5848)

@miri64 miri64 merged commit 7974bf2 into RIOT-OS:master Nov 4, 2016
@miri64 miri64 deleted the tinydtls/fix/buildsystem branch November 4, 2016 17:07
@miri64 miri64 mentioned this pull request Nov 4, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OlegHahm OlegHahm OlegHahm approved these changes

@kaspar030 kaspar030 kaspar030 left review comments

Assignees

@OlegHahm OlegHahm

Labels
Area: network Area: Networking 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: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
Release 2016.10
Development

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