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

Update for package distribution #955

Merged
merged 11 commits into from May 19, 2020
Merged

Update for package distribution #955

merged 11 commits into from May 19, 2020

Conversation

Nightwalker-87
Copy link
Member

@Nightwalker-87 Nightwalker-87 commented May 10, 2020

Changes & additions:

  • New cpack package-config for DEB and RPM
  • Toolchain file for cross-building
  • Script to automate MinGW cross-building
  • Added file headers for .cmake files
  • Added dpkg and rpm to required pkgs
  • Removed deprecated debian pkg config settings
  • Update for travis build configuration:
    --> Added builds for clang -m32 on arch: AMD64
    --> Corrected invalid config settings
    --> Added clang-9 builds for linux
    --> Moved scripts to root directory
    --> Added MinGW cross-test-build for linux
  • Updated steps for release preparation
  • Changed destination for binary archives
  • Added note on windows binary installation (Closes [install] Windows binary is configured to a hard-coded path #738) (Closes [doc] Windows binary installation instructions? #795)
  • Contributors now listed in separate file
  • Added version info for static library
  • Added package changelog files for deb & rpm
  • Added license & rules file for deb pkg
  • Removed unneeded commands in travis.sh
  • Removed -Wno-dev flag in Release target
  • cmake: Grouped udev and modprobe.d config
  • rpm package now relocatable (CPackRPM)
  • Tidy up install paths & directories:
    -> modprobe.d & udev rules (both are now ON per default on Linux)
    -> stlink-gui supplements
    -> man pages
  • Fixed paths according to GNUInstallDirs
  • cmake: Restructured library config
  • Test files & gui now use shared library

- New cpack package-config for DEB and RPM
- Toolchain file for cross-building
- Script to automate MinGW cross-building
- Added file headers for .cmake files
- Added dpkg and rpm to required pkgs
- Added builds for clang -m32 on arch: AMD64
- Corrected invalid config settings
- Added clang-9 builds for linux
- Moved scripts to root directory
- Added MinGW cross-test-build for linux
- Updated steps for release preparation
- Changed destination for binary archives

(Closes #798)
Copy link
Collaborator

@slyshykO slyshykO left a comment

Choose a reason for hiding this comment

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

It is Ok for me

@Nightwalker-87
Copy link
Member Author

Nightwalker-87 commented May 10, 2020

It is Ok for me

Wait - it's not ready yet ... 👀
... but you can do a cross-build on a debian-based system and unzip the windows binary on a windows installation to see if everything works.

Release v1.6.1 automation moved this from In progress to Reviewer approved May 10, 2020
@slyshykO
Copy link
Collaborator

Wait - it's not ready yet ...

Something else planned?

@Nightwalker-87
Copy link
Member Author

Only some minor fixes, that's why I opened a draft PR.

- Added note on windows binary installation
- Contributors now listed in separate file

(Closes #738) (Closes #795)
@Nightwalker-87
Copy link
Member Author

As @slyshykO reported, there is still an issue with a static linking to libssp on Windows when the binary package is generated via a debian cross-build which is not as expected. This needs a fix.

When building natively on Windows this issue does not occur, as I could verify by myself on a virtual machine. As of the current state of knowledge I can't make out any difference in the related cmake compilation routine in CMakeLists.txt.

If the reason is not found and nobody else is going to show any interest in this, cross-building for Windows will only generate non-functional binaries and thus will be useless...

- Simplified code
- Added version info for static library
@Nightwalker-87
Copy link
Member Author

Nightwalker-87 commented May 18, 2020

Well they can do as they like, but I am not willing to generate several rpm packages for a project release. Thus this package should be widely deployable.
I think I have also found a solution for the above in the meanwhile.

@Vascom
Copy link
Collaborator

Vascom commented May 18, 2020

You can not generate any.

@Nightwalker-87
Copy link
Member Author

Nightwalker-87 commented May 18, 2020

You can not generate any.

We already do - with cmake ;-)

- Removed unneeded commands in travis.sh
- Removed -Wno-dev flag in Release target
- cmake: Grouped udev and modprobe.d config
- rpm package now relocatable (CPackRPM)
@Nightwalker-87
Copy link
Member Author

@slyshykO @chenguokai: These were the outstanding changes still missing to conclude this PR.
Now this static linking of libssp is the last nut to crack...

@Vascom
Copy link
Collaborator

Vascom commented May 18, 2020

You can not generate any.

We already do - with cmake ;-)

I mean do it properly as every distro recommend and require.

@Nightwalker-87
Copy link
Member Author

Nightwalker-87 commented May 18, 2020

That's why there are package maintainers, but that should not prevent us from providing a general-use rpm package here on our GitHub release page, as we do provide one for debian-based systems. Can we conclude this topic now? This is also not the right place to discuss this, BTW.

@Vascom
Copy link
Collaborator

Vascom commented May 18, 2020

Can we conclude this topic now? This is also not the right place to discuss this, BTW.

Yes.

Tidy up install paths & directories:
-> modprobe.d & udev rules
  (both are now ON per default on Linux)
-> stlink-gui supplements
-> man pages
- Fixed paths according to GNUInstallDirs
- cmake: Restructured library config
- Test files & gui now use shared library
@Nightwalker-87
Copy link
Member Author

@chenguokai Can you give it a test again on macOS? I've successfully compiled, installed and used the toolset executables on debian.
@slyshykO: Linking to libssp-0.dll on windows for cross-build binaries is still non-functional. I've decided not to fix this within this PR any more. It should be addressed separately. I may also leave that to others.

If there are no further significant issues I'd like to merge this PR now. It has become quite bulky already.

@slyshykO
Copy link
Collaborator

slyshykO commented May 19, 2020

@Nightwalker-87
Can link libssp statically by changing only one line in the project
3d34ea5

@Nightwalker-87
Copy link
Member Author

Nightwalker-87 commented May 19, 2020

Can link libssp statically by changing only one line in the project

Oh that is good news - I'm surprised it is achievable that easy. I tried that argument somewhere else before, what didn't work. Ok - wait let us finish and merge this PR first. Yours should follow subsequently then.

@slyshykO
Copy link
Collaborator

I'm surprised it is achievable that easy.

Looks easy but I, like you, tried many times before getting success on build :)

@chenguokai
Copy link
Collaborator

chenguokai commented May 19, 2020

Edit: the first one is resolved on develop branch so not a problem actually. The below one, in my view, can be ignored for now. I will take care of it after merge.

Mostly good with one problem.

1.. cmake failed with with a simple version bump in MacPorts Portfile

...
-- Found libusb: /opt/local/lib/libusb-1.0.a  
-- Could NOT find PkgConfig (missing: PKG_CONFIG_EXECUTABLE) 
-- Looking for sys/mman.h
-- Looking for sys/mman.h - found
-- Looking for unistd.h
-- Looking for unistd.h - found
-- Looking for __stack_chk_fail in ssp
-- Looking for __stack_chk_fail in ssp - not found
-- STLINK_LIB_SHARED: stlink
-- PROJECT_VERSION_MAJOR: 
-- VERSION: ..
CMake Error at CMakeLists.txt:167 (set_target_properties):
  set_target_properties called with incorrect number of arguments.


-- STLINK_LIB_STATIC: stlink-static
-- PROJECT_VERSION_MAJOR: 
-- VERSION: ..
CMake Error at CMakeLists.txt:203 (set_target_properties):
  set_target_properties called with incorrect number of arguments.
...

The one could not be a problem since I only checked out dist branch which could have missed out some commits.

@Nightwalker-87
Copy link
Member Author

  1. Try:
    execute_process(COMMAND bash -c "export LD_LIBRARY_PATH="${CMAKE_INSTALL_LIBDIR}" ")
    (Note the change a the end!)

  2. Here your version numbers were not read correctly, which is the reason why it failed. Git present? Version file present with the correct format? (This is not a big deal though, looks like something local, because I fixed that before.)

@Nightwalker-87 Nightwalker-87 merged commit da68271 into develop May 19, 2020
Release v1.6.1 automation moved this from Reviewer approved to Done May 19, 2020
@Nightwalker-87 Nightwalker-87 deleted the dist branch May 19, 2020 14:38
@Nightwalker-87
Copy link
Member Author

Thank you both @chenguokai & @slyshykO for reviewing this. 🥇

@chenguokai
Copy link
Collaborator

chenguokai commented May 19, 2020

Try: execute_process(COMMAND bash -c "export LD_LIBRARY_PATH="${CMAKE_INSTALL_LIBDIR}" ") (Note the change a the end!)

Finally I figured out the problem here. Might be a MacPorts specific issue with one cmake flag set like -DCMAKE_MODULE_PATH="/opt/local/share/cmake/Modules" where the path /opt/local/share/cmake/Modules does not exist. I will ask the developers of MacPorts for more information about the path.

Edit: Not triggered by this, I forgot to remove my local modification before testing😅

Maybe we should ignore any external CMAKE_MODULE_PATH.

@Nightwalker-87
Copy link
Member Author

Edit: Not triggered by this, I forgot to remove my local modification before testing 😅

That will likely help... 😄

@stlink-org stlink-org locked as resolved and limited conversation to collaborators Jun 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Status: Done
4 participants