RFC: Build libstlink shared library, add debian packaging

[nekromant]

PLEASE DO NOT MERGE YET!

Well, basically sometimes one may need to use stlink in his own app. This patchset does the following:

1. Adds a shared library target and links all binaries against it, including proper SOVERSION stuff
2. Installs all the relevant headers to their respective locations
3. Teaches CMakeLists.txt to properly handle ${CMAKE_LIBRARY_PATH} and play nicely with debian multiarch
4. Teaches CMakeLists.txt to generate and install a proper pkg-config file
5. Add initial debian packaging stuff to debian/ directory

As of debian packaging, I've split the package into three:
libstlink - the shared library
libstlink-dev - pkg-config file, static library and headers
stlink-tools - all the binaries

Please don't yet merge, since there are a few things I haven't yet tested out/firgured.

1. I've added a few CMake variables that set .so version. I've set API version as 1 and library version an 1.2.1 (for the current git master). Correct me if I'm wrong here.
2. I'm no big autotools expert, so I've done nothing so far with autotools stuff.
3. I've only tested building under linux, so my changes to CMakeLists.txt MAY have broken windows/mac builds.

[xor-gate]

Looking good, is the debian folder compatible with debian itself? Keep this in mind: LWN - The case against upstream packaging. First pull request #434 needs to be merged because autotools will be deprecated as we support Mac/Linux/Windows & probably works under BSD and autotools is a pain under windows and mac.

[xor-gate]

I have already cleaned up and extended the CMakeLists.txt file (in the texane/stlink -deprecate_autotools branch and it automatically fetches the version if the git HEAD pointer points at a tag or when a zipfile is used it is loaded from a .version. Which is inside the pull request #434.

[nekromant]

Looking good, is the debian folder compatible with debian itself? Keep this in mind: LWN - The case against upstream packaging.

Yeah, I've seen that one, it's mostly about those fancy 'snap' packages and I completely agree that they are evil (and should die along with those who propose them). On the contrary I don't think that giving people a simple way to run dpkg-buildpackage and getting a deb for their system will hurt in any way related to maintaining a package. In our case debian/ubuntu maintainers will just have one less patch in their batch.

The pros are that things will get simpler for devs when it comes to providing nightly debian builds for testing and/or deploying the package to your own hardware. The problem maintaining software like stlink IMO is that it's hardware related and it is intended for developers. And if ST releases, say, stlink v3 a lot of people with new hardware will forget all the maintainers' work and compile from source anyway.

I have already cleaned up and extended the CMakeLists.txt file (in the texane/stlink -deprecate_autotools branch and it automatically fetches the version if the git HEAD pointer points at a tag or when a zipfile is used it is loaded from a .version. Which is inside the pull request #434.

Okay, should I wait for texane/stlink -deprecate_autotools branch to be merged into master, or just rebase this patchset on that branch?

P.S. I think I can help out a little bit with CI ( Like I do it here: https://jenkins.ncrmnt.org/job/GithubCI/job/aura/ ) and automatic packaging. I've mostly set up an infrastructure that builds and manages a debian repository from within jenkins and provides debs in batch for armel/armhf/amd64/i386/aarch64 and publishes them in a repository. I still need to polish a few bits and pieces, but it should be live soon.

[xor-gate]

@nekromant I really appreciate you work on CI packaging and improving but I also had enabled AppVeyor for windows (mingw builds) and had some discussion with @texane about the forward progress here #437, and here PR 434 - discussion. And I put my major effort currently in similar project stlink2 which will be available soon with different requirements and MIT licensed.

[nekromant]

I don't think AppVeyor really hurts, since my jenkins instance has no windows nodes and right now I don't have the resources to add any. But I think I can help out with packaging once I have all set up.

Right now I can't keep actual hardware connected since I have only 2 USB ports that have individual power control and both are busy for CI-testing my RPC library called aura. In the upcoming months I hope to get the PCBs for my homebrew USB hub that will have 6 ports with individual power control, so I can add some hardware (I have 2 different discovery boards with v1 and v2 stlink). Since I plan to add RPC transport over stlink (that's why I've started this PR after all) I'll have at least one of those boards rigged to my jenkins all the time anyway.

[nekromant]

P.S. Here you go: https://jenkins.ncrmnt.org/job/GithubCI/job/stlink/

[nekromant]

Okay, I've tested all that I could and added a standalone application example. You are free to merge whenever you want.

P.S. I also rigged clang-analyze to do some static analysis and it uncovered some bugs: https://jenkins.ncrmnt.org/job/GithubCI/job/stlink/2/clangScanBuildBugs/

[xor-gate]

@nekromant looks good, if you want a "real CI" we must have the following steps:

• Linting (codestyle enforcement, linux checkpatch or astyle)
• Static analyzer (clang-analyzer, oclint)
• Code coverage (gcov, govr with cobertura xml output for jenkins)
• Runtime checker (valgrind)
• Unit testing (unity, google test)
• Code cyclomatic complexity checker (oclint, terryin/lizard, metrixpp)

[xor-gate]

@nekromant I'm already aware of those static analyzer found bugs. I have run stlink code also on our own jenkins CI platform which I saw them pop up in the graphs. As you can see, the gdb server stubs are in a very bad shape.

[xor-gate]

I'm not a fan putting auxiliary scripts in the root, you better off to place them in a scripts folder and add a small note what it does. You can also feed cmake a different compiler from a top-level Makefile.

[xor-gate]

I have not yet tested this, but as texane probably disabled travis builds for OSX on his personal account we can not verify it has not been killed. And you should forget autotools, as most beard-wearing people also switching to CMake which care about true mac and windows builds.

[nekromant]

Okay. Let me know if anything else in this PR needs fixing.
As for CI my checklist that I've set up is as follows:

• ctest for unit-testing, since it's easy to integrate with jenkins and comes free with CMake.
• Coveralls for measuring code coverage, upload from jenkins. So far the easiest I've found to set up.
• The test-suite is run twice, during second run is the test is run under valgrind. If memory leaks - test is assumed as failed, reports are all aggregated by jenkins.
• checkpatch.pl for coding style... Well, I'm a kernel guy, so that's obvious for me ;)
• coverity scan for static analysis (way more serious than clang-alanyze).

[xor-gate]

• CTest is just a runner, no unit-testing framework (Unity in C89, Google test C++)
• Checkpatch.pl FTW then :+), still it is C-code and no style has been established so obvious choice would be linux kernel codestyle as there is already a document and tool
• Coverity is nice but very obsecure to see/login for others, but I have some bugs seen only detected by clang-analyzer.
• Coveralls is nice, but jenkins has support with some plugins to have integrated code coverage (need to lookup how I have this setuped)

[nekromant]

Unity for C89 is damn monster! Thanks for telling me about it ;). I've worked a little bit with ctest (this ctest: https://github.com/bvdberg/ctest ), but for my purposes I ended up writing tests as simple applications with no testing framework, since they can also serve as ready to use examples. I haven't been developing things that big that will require such testing frameworks. Using that stuff for stlink... Well, for me it looks like using a nuke to squash a roach ;)

[xor-gate]

I understand, but if you create more complex unit tests which need to produce good code coverage using a common framework is suggested. As C language has no "testable example" as in Golang, you could add a examples folder and compile and run it with CMake CTest.

[xor-gate]

Hi @nekromant I have merged PR #434 could you rebase your work on master and squash all the work into one commit. Then we can review and merge this also to master.

Thank you for your contribution!

[nekromant]

@xor-gate Got it, will get to it in a day or two.

[xor-gate]

I will wait until you are in sync with master. When you are finished I will prepare for release of version 2.0.0.

[nekromant]

Okay, I think I've done it. I've added some additional PROJECT_VERSION handling to properly set it (and soversion) from STLINK_PACKAGE_VERSION. You may want to tripple check the logic. I think I got it right. I've also bumped debian changelog to version 2.0.0.

[xor-gate]

Looks good but I have some comments:

• Don't mix highercase and lowercase in CMake function call syntax, only use lowercase e.g (set() instead of SET()). To keep it consistent
• PROJECT_API_VERSION is unnessary as we use SemVer and use the PROJECT_VERSION_MAJOR to signal API version (See https://cmake.org/cmake/help/v3.0/variable/PROJECT_VERSION_MAJOR.html).
• Why are we creating a debian folder, when this wont go upstream into Debian then we should probably get rid of this and use CPack to create debian packages. Because the project itself can not keep up with all the packaging details necessary for all the operating systems and linux variants. I'm not against having a debian folder inside the repository but most people dont even care or know how to build a debian package.
• Could you move the app-example into doc/example as this pollutes the root directory (Still it is nice to have it compiling in debug builds).
• The pkg-config should only be generated for platforms where it is available. On Windows/OSX it is not available by default. You should move the generation of this file to <root>/usr/lib/pkgconfig/<template>.in and the whole cmake directives into a seperated CMakeLists. So the top-level cmake is not polluted (See https://github.com/texane/stlink/blob/57cf59a86930b37d0d408fc58b6f13b44c8a431a/CMakeLists.txt#L6, and https://cmake.org/Wiki/CMake:How_To_Find_Libraries). Then you can wrap the generation of the .pc file into a `if (PKGCONFIG_FOUND).

[nekromant]

Why are we creating a debian folder, when this wont go upstream into Debian

Adding debian/ stuff has the following rationale:

• As far as I know, cpack can't create several packages (e.g. -dev, -doc, -gui) out of one project tree (correct me if I'm wrong here). In case of stlink we have the gui that requires a lot of dependencies that are totally useless on, say, a headless system. Currently implemented packaging takes case to separate things as appropriate.
• We can build the package painlessly in jenkins using debian-package-builder plugin (With some additional plugins and software (docker, aptly, skyforge) we can package it for several architectures in one run and provide a ready-to-use repository. This basically what we're doing for our software at @RC-MODULE and what I want to setup for my own software and tools asap.
• Same as above, this simplifies integration with OpenSuse OBS. (Although I've had some very unpleasant experience with OBS and ended up using jenkins for all the packaging)

I completely agree that we can't package it for every linux distro out there, but adding support for some widely used distros could save some time. Whether to include this one or not is completely up to you.

As for the rest, I'll try to fix it this week

[xor-gate]

I agree on your points, I'm also not sure how to create multiple packages from one source-tree with CMake/CPack as I have never did this. And the approach you did seems the less worse 👍 . When you have resolved those comments we are ready to go (don't forget to rebase-squash into a single commit).

[nekromant]

@xor-gate Sorry for the delay, I've finally got time to fix the PR and ran into some tricky points, namely:

Don't mix highercase and lowercase in CMake function call syntax, only use lowercase e.g (set() instead of SET()). To keep it consistent

fixed

PROJECT_API_VERSION is unnessary as we use SemVer and use the PROJECT_VERSION_MAJOR to signal API version (See https://cmake.org/cmake/help/v3.0/variable/PROJECT_VERSION_MAJOR.html).

The version stuff is tricky, since version detection here relies on variables set by project() directive, but as of cmake policy CMP0048 we're expected to set all the version variables from project(). Oops. As of now I've resorted to using STLINK_PACKAGE_VERSION_* variables for everything. soversion should also now be set properly for windows builds.

Could you move the app-example into doc/example as this pollutes the root directory

done

(Still it is nice to have it compiling in debug builds).

Adding it as a subproject makes it link against system-installed stlink, if any. And I don't want to bloat the example's CMakeLists.txt with any logic - just keep it bare minimum. Note, I haven't tried building it under mingw/msys for I can't test it right now.

The pkg-config should only be generated for platforms where it is available. On Windows/OSX it is not available by default.

Fixed.

You should move the generation of this file to /usr/lib/pkgconfig/.in

Can you clarify that a little? Should I just move pkf-config.cmake to ${CMAKE_SOURCE_DIR}/usr/lib/...?

and the whole cmake directives into a seperated CMakeLists

After looking through the code, I don't seem to find a more or less reasonable way to move away the pkg-config generation logic, since wrapping it in a macro of function will get us nearly as many lines of arguments as we have now.

Then you can wrap the generation of the .pc file into a `if (PKGCONFIG_FOUND).

The .pc generation logic does not rely on pkg-config present in the system. It's just find-replace done by cmake. In fact, I don't think it will hurt having this file along with windows builds, but it's your decision to make.

I'll squash all the commits in one once you say it's all good ;)

[xor-gate]

Please separate ${PROJECT_NAME}static with a -. Maybe better would be to have a variable like STLINK_LIB_STATIC, STLINK_LIB_SHARED for the target names.

[nekromant]

I've fixed the remaining things in both CMakeLists.txt. See if everything's okay now. If it is, I'll squash everything in a single commit so that you can finally merge.

[xor-gate]

cmake_minimum_required should always be the first line for CMakeLists.txt

[nekromant]

cmake_minimum_required should always be the first line for CMakeLists.txt

It is there on the first line, diff just looks a bit weird due to adding sidenotes.

[xor-gate]

You should replace all occurences of ${PROJECT_NAME}-static target with:

${STLINK_LIB_STATIC}

and should be set once in the main CMakeLists.txt:

set(STLINK_LIB_STATIC ${PROJECT_NAME}-static)

[xor-gate]

ping @nekromant

[nekromant]

@xor-gate Sorry, I've been out of the city, just got back to fix it. I've merged all my work in a single commit so that it would be easier to review what's left of it.

[xor-gate]

Hi @nekromant I was a weekend AFK and I will review the whole bunch next week completely. Thank you for your work!

[xor-gate]

In upcoming time I will refactor some things and prepare a new release. Do you mind if I add your build (jenkins) badge also to the readme? Maybe you could poll texane/stlink?

[nekromant]

No problem, I've just set up jenkins to use texane/stlink for building. Can you set up the github to ping my jenkins instance when something is pushed?

P.S. I will rig a real stlink-v1 board to jenkins in the upcoming months. Just have to find some time and lay out a proper usb hub with per-port power management.

[texane]

Hi Jerry, are there side effects in doing so ? If not, please do it.

2016-09-07 21:11 GMT+02:00 Jerry Jacobs [email protected]:

In upcoming time I will refactor some things and prepare a new release. Do
you mind if I add your build (jenkins) badge also to the readme?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#444 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAEeH7-9oIrkd1AZCxFIXe98sZ9qVJbAks5qnwxLgaJpZM4JItUD
.

[xor-gate]

Hi all, no I can not send a webhook when something is pushed. As texane/stlink project administartion is owned by texane himself. It is now showstopper whatsoever. Because you can still let jenkins poll every X minutes for new commits, when a commit has changed it will build.

I use this also at my works this way, without the need to have hooks configured. If you need help with jenkins. I have proffesional experience.

Hooking up a board and have some code coverage displayed would be really really awesome. We need to see how this fits as I dont want any CI-related scripts in the main repo. Probably we should have a seperate repository somewhere else which is used to run on real hardware.

I prefer a custom jenkins with e.g Debian because we indeed can hook up some real hardware.

[nekromant]

Thanks, I've got quite some experience with jenkins as well, had to even dive into plugin development a few times. No problem here ;)
I still prefer to use webhooks when possible instead of polling, no lag that way. (Right now it polls the repo every 30 minutes or so).

If you want, I can set up the instance to build branches using Jenkinsfile, this way you can keep all the CI stuff in the repo and not rely on me to fix a thing or two (Just, please, don't do rm -Rfv / or mine bitcoins;) )

[xor-gate]

Wow, thats getting your hands dirty. A Jenkinsfile is nice. Mining bitcoins will not happen as this takes way to long LOL. And rm -Rfv is also not a good idea. Having the seperate Jenkinsfile is a good idea. Maybe you could draft one (where clang-analyzer also works). I have tried to setup a pipelined build with a jenkins build and was not able to use all job features.

[nekromant]

Jenkins DSL is somewhat new and new features are coming with every release, so you might've missed them the day you tried. I'll give it a spin, but can't make any promises. I'm just back to work from the vacation and things are quite hellish right now. Will post to a new issue.

[xor-gate]

I have created a issue where we can discus further in #461. Locking this PR so this wont get polluted 👍

[Nightwalker-87]

This PR is now finally closed.