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

add infrastructure for binary LGPL compliance checks #2362

Merged
merged 2 commits into from
Mar 30, 2016
Merged

add infrastructure for binary LGPL compliance checks #2362

merged 2 commits into from
Mar 30, 2016

Conversation

kaspar030
Copy link
Contributor

generalized version of #2357.
Work in progress. beauty-patches welcome! README.md outdated.

This PR adds some new make targets:
patch -> apply all patches in patches/ folder to $RIOTBASE using git am
bindist -> creates a folder "bindist" which contains all files needed for LGPL-compliant distribution
check_bindist -> supposed to be run from within a folder created by "make bindist". Checks if compiled RIOTBASE linked with included .a files md5-matches included .elf.

Please check out the concept before I thorougly document it. ;)

@kaspar030 kaspar030 added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: new feature The issue requests / The PR implemements a new feature for RIOT CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels Jan 27, 2015
@kaspar030 kaspar030 mentioned this pull request Jan 27, 2015
Closed
@OlegHahm OlegHahm force-pushed the master branch 3 times, most recently from 9f184dd to 45554bf Compare March 31, 2015 13:01
@emmanuelsearch
Copy link
Member

@kaspar030 so where do we stand on this one?

Kijewski
Kijewski reviewed Apr 24, 2015
View reviewed changes
Makefile.base
${DIRS:%=ALL--%}:
"$(MAKE)" -C ${@:ALL--%=%}
"$(MAKE)" -C ${@:ALL--%=%} || [[ "$(WEAK_DIRS)" != *${@:ALL--%=%}* ]]
Copy link
Contributor

Choose a reason for hiding this comment

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

The test is in the wrong order, isn't it?

@OlegHahm OlegHahm modified the milestone: Release 2015.06 Apr 29, 2015
@OlegHahm
Copy link
Member

@kaspar030 so where do we stand on this one?

@kaspar030 kaspar030 modified the milestones: Release NEXT MAJOR, Release 2015.09 Sep 2, 2015
@OlegHahm
Copy link
Member

@kaspar030?

@OlegHahm
Copy link
Member

OlegHahm commented Dec 4, 2015

🍺 ?

@kaspar030 kaspar030 force-pushed the add_lgpl_compliance_simplified branch from 5ae32f7 to 0cdf9ee Compare December 4, 2015 11:26
@kaspar030
Copy link
Contributor Author

I rebased it, but the make targets don't work. I'll have no time until monday to investigate.
And, this is still RFC in terms of concept, noone commented...

@kaspar030 kaspar030 removed this from the Release 2015.12 milestone Dec 4, 2015
@OlegHahm OlegHahm modified the milestones: Release 2015.12, Release 2016.03 Dec 8, 2015
@emmanuelsearch
Copy link
Member

Would be awesome to have some progress on this...

@kaspar030 kaspar030 force-pushed the add_lgpl_compliance_simplified branch from 0cdf9ee to 362d999 Compare March 16, 2016 11:06
@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 16, 2016
@kaspar030 kaspar030 removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Mar 16, 2016
@kaspar030
Copy link
Contributor Author

  • rebased
  • dropped library example. @BytesGalore, it seemed too complicated, and I'm not even sure it's concept doesn't make the whole a derived work.

@emmanuelsearch
Copy link
Member

So is this still WIP, or not?

@Kijewski
Copy link
Contributor

Maybe $RANDOM_PERSON could have a look how the Debian folks implemented their reproducible builds: https://wiki.debian.org/ReproducibleBuilds Probably wrong PR for my comment.

@emmanuelsearch
Copy link
Member

I think this new feature is great!

@emmanuelsearch
Copy link
Member

Tested. I get this error. Is there a file missing in the commits?

RIOT/Makefile.bindist:4: Makefile.distcheck: No such file or directory
make: *** No rule to make target 'Makefile.distcheck'. Stop.

(I was not executing from the folder generated by bindist ;)

@emmanuelsearch
Copy link
Member

A separate OS X issue: on Mac I get this error at when I make bindist:

In file included from /RIOT/cpu/native/periph/timer.c:42:
In file included from /RIOT/cpu/native/include/native_internal.h:177:
/RIOT/core/include/sched.h:97:24: error: typedef
      redefinition with different types ('struct _thread' vs 'mach_port_t' (aka 'unsigned int'))
typedef struct _thread thread_t;
                       ^
/usr/include/mach/mach_types.h:119:22: note: previous definition is here
typedef mach_port_t             thread_t;
                                ^
1 error generated.

@BytesGalore
Copy link
Member

@kaspar030 I guess you're right, the library linking example is something ontop of the base functionality of this PR.
I still think this PR is very valuable to help/enable easy checking of LGPL compliance for shipped binary blobs.

@emmanuelsearch
Copy link
Member

@kaspar030 for some reason it keeps telling me Warning! RIOT_VERSION doesn't match! and the .elf don't match even though I am using the same commit and the same toolchain, when I make check_bindist
I suppose I need to patch at some point, and that's the reason? But what, how and when?

@kaspar030
Copy link
Contributor Author

Hm, RIOT_VERSION doesn't match is weird.

Can you try adding RIOT_VERSION_OVERRIDE:=test to the makefile, remove bindist, then make clean all again?

@emmanuelsearch
Copy link
Member

Tested, works even without the RIOT_VERSION_OVERRIDE:=test
I think the README should be modified to explain that the process is:

  1. make all
  2. make bindist
  3. cd bindist
  4. make check_bindist

Is there also a way to automate that the RIOTBASE in the bindist/Makefile automatically appends a /.. by default? (if a user needs to update it nevertheless, it does not change anything, else it saves one manual edit ;)

@emmanuelsearch
Copy link
Member

@kaspar030 so what's the latest on this? Any chance this makes the feature freeze?

@kaspar030
Copy link
Contributor Author

I think the README should be modified to explain that the process is:

updated.

Is there also a way to automate that the RIOTBASE in the bindist/Makefile automatically appends a /.. by default?

Hm, in principle, but other than make the example work, it would be fiddling with a user's makefile. I've added the step to the README, do you think this is enough?

so what's the latest on this? Any chance this makes the feature freeze?

IMHO we can merge, if you give it an ACK. ;)

@emmanuelsearch
Copy link
Member

ACK!

@emmanuelsearch emmanuelsearch added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 29, 2016
@kaspar030 kaspar030 force-pushed the add_lgpl_compliance_simplified branch from 6b8968b to 11a4391 Compare March 29, 2016 20:47
@kaspar030
Copy link
Contributor Author

  • squashed, rebased, dropped the unneeded patch target commit, waiting for murdock

@OlegHahm
Copy link
Member

lol:

file has an unknown license header: 'examples/bindist/abc/abc.c'

I'm fine with merging this anyway. ;)

@emmanuelsearch
Copy link
Member

@kaspar030 can you add license + copyright P. Schmerzl on abc.c? ;)
should be good to go

@kaspar030 kaspar030 force-pushed the add_lgpl_compliance_simplified branch from 11a4391 to 1a003eb Compare March 30, 2016 09:01
@OlegHahm OlegHahm merged commit 506e23f into RIOT-OS:master Mar 30, 2016
@kaspar030 kaspar030 deleted the add_lgpl_compliance_simplified branch February 7, 2017 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@emmanuelsearch emmanuelsearch

Labels
CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Milestone
Release 2016.04
Development

Successfully merging this pull request may close these issues.
5 participants
@kaspar030 @emmanuelsearch @OlegHahm @Kijewski @BytesGalore