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

Use submodules and subdirs #294

Merged

Conversation

OPNA2608
Copy link
Member

@OPNA2608 OPNA2608 commented Dec 9, 2020

(targeting link_rtaudio_rtmidi branch for lower code delta)

  • Rt{Audio,Midi} have been deleted from the repository & re-added as git submodules.
  • A root-level QMake project file "Project.pro" (good enough name-wise, though open for suggestions) has been added. It uses the subdirs template to check & build the sub-projects:
    • BambooTracker (C++ code for the tracker itself)
    • data (assets that aren't compiled into object files)
    • RtAudio (handles feature detection for & statically compiling of RtAudio, disabled by passing CONFIG+=system_rtaudio)
    • RtMidi (handles feature detection for & statically compiling of RtMidi, disabled by passing CONFIG+=system_rtaudio)
  • The resource pack data (/data/resources/) has been moved to /BambooTracker. While it's technically better in /data, it's being compiled into an *.o file which doesn't fit well with how the rest of the assets are being treated (no compiling & linking into the BambooTracker binary required, so using compiler-less TEMPLATE option) and makes including the object file into BambooTracker abit harder than when everything was mashed together. Open for suggestions here.
  • Added automatic feature detection to the Rt{Audio,Midi} sub-projects. A series of qtCompileTests is used to determine what APIs are supported by the system, similar to how Rt{Audio,Midi}'s own build system detects available features.

needs to be retargeted at master after #288 is merged See message below.

@OPNA2608 OPNA2608 changed the base branch from master to link_rtaudio_rtmidi December 9, 2020 22:31
- Rt{Audio,Midi} -> git submodules
- global project file Project.pro, uses sub-projects via subdirs template
- resource pack moved to BambooTracker (necessitated by being compiled into binary object format)
- use_* complemented via feature detection in Rt{Audio,Midi} sub-projects
Wrote & use common API test function, evaluate use_* config variables
against platform-required APIs and detected APIs to error on missing
dependencies.
@OPNA2608
Copy link
Member Author

Updated testing to take the platform's required & specifically enabled APIs into consideration, erroring out early of either of them cannot be satisfied. Needs more changes to disable all this testing if system_rt* is passed.

@OPNA2608 OPNA2608 marked this pull request as ready for review December 19, 2020 15:23
@OPNA2608 OPNA2608 changed the base branch from link_rtaudio_rtmidi to master December 19, 2020 15:24
@OPNA2608 OPNA2608 changed the base branch from master to link_rtaudio_rtmidi December 19, 2020 15:28
@OPNA2608
Copy link
Member Author

@rerrahkr I consider this basically-done now, I'd appreciate if you could try to compile & run this on Windows & possibly macOS. I've struggled abit with the added git submodules at the start, I hope they're not giving you any problems (might need a git submodule init && git submodule update after checking out for submodules/Rt{Audio,Midi}/src to get the repo contents).

I don't know which branch to merge this into - it's based on link_rtaudio_rtmidi but doing it like this won't run any build tests, merging it in master would be better for the added github & appveyor builds but there are merge conflicts I'll have to look into first, some of which may be inherited from your branch.

For the time being, I'll push this branch to my fork's master and push a tag there, bearing in mind that the XP builds will fail due to #296.

@OPNA2608
Copy link
Member Author

GitHub CI: https://github.com/OPNA2608/BambooTracker/runs/1581887247
AppVeyor CI: https://ci.appveyor.com/project/OPNA2608/bambootracker/builds/36921263
Release: https://github.com/OPNA2608/BambooTracker/releases/tag/v0.0.0

macOS on GitHub failed, looks like a problem with Homebrew (which I didn't really touch since the last run). Unfortunate, I hope that fixes itself without me having to touch the Homebrew stuff. 😕

@rerrahkr
Copy link
Member

I will test them on mac later.

When I run the Windows build of Appveyor, I get an error that Qt5Core.dll and Qt5Gui.dll are missing.
It seems that there are two Qt dlls, one for release (Qt5Core.dll) and one for debugging (Qt5Cored.dll) (found it here), so the release build setting may be enabled.

.appveyor.yml Outdated Show resolved Hide resolved
@OPNA2608
Copy link
Member Author

OPNA2608 commented Dec 22, 2020

Hmm nevermind, that didn't fix it… I'll dig out my Windows machine and try to figure out where those dependencies on release libraries are coming from. 😕

Before: (5449f96 - https://ci.appveyor.com/project/rerrahkr/bambootracker/build/job/lns339ixv7ual5sl)

readPeExecutable: C:\projects\bambootracker\target\BambooTracker.exe 32 bit, MinGW, dependent libraries: 12, debug
readPeExecutable: C:\Qt\5.5\mingw492_32\bin\Qt5Cored.dll 32 bit, MinGW, dependent libraries: 11
readPeExecutable: C:\Qt\5.5\mingw492_32\bin\Qt5Guid.dll 32 bit, MinGW, dependent libraries: 7
readPeExecutable: C:\Qt\5.5\mingw492_32\bin\Qt5Widgetsd.dll 32 bit, MinGW, dependent libraries: 9

After:

readPeExecutable: C:\projects\bambootracker\target\BambooTracker.exe 32 bit, MinGW, dependent libraries: 12, debug
readPeExecutable: C:\Qt\5.5\mingw492_32\bin\Qt5Core.dll 32 bit, MinGW, dependent libraries: 11
readPeExecutable: C:\Qt\5.5\mingw492_32\bin\Qt5Gui.dll 32 bit, MinGW, dependent libraries: 7
readPeExecutable: C:\Qt\5.5\mingw492_32\bin\Qt5Widgetsd.dll 32 bit, MinGW, dependent libraries: 9
readPeExecutable: C:\Qt\5.5\mingw492_32\bin\Qt5Cored.dll 32 bit, MinGW, dependent libraries: 11
readPeExecutable: C:\Qt\5.5\mingw492_32\bin\Qt5Guid.dll 32 bit, MinGW, dependent libraries: 7

@rerrahkr
Copy link
Member

I will test them on mac later.

I tested them a while ago, and they worked fine.

@OPNA2608 OPNA2608 changed the title WIP Use submodules and subdirs Use submodules and subdirs Dec 22, 2020
@OPNA2608
Copy link
Member Author

OPNA2608 commented Dec 23, 2020

GitHub CI: https://github.com/OPNA2608/BambooTracker/commit/0f3ea09f8b4ea050b173284773780078c9759d15/checks
AppVeyor CI: https://ci.appveyor.com/project/OPNA2608/bambootracker/builds/37003008 (uploads failed, run out of artifact space)
Same release. Shadow building & subsequent linking against Rt* libs fixed, Nixpkgs CI stability improved.

Comment on lines +62 to +66
# JACK implies brew link python-3.9, fails due to shipped python binaries
brew unlink python@3.9
brew link --overwrite python@3.9
brew upgrade --force python@3.9
brew install qt5 pkg-config jack p7zip
Copy link
Member Author

Choose a reason for hiding this comment

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

jack has a dependency on python@3.9. When it tries to install python-3.9.1_1 on GHA and link its binaries into the system's common bin path, it finds a symlink/binary(?) in /usr/local/bin that belongs to macOS' shipped python2 installation and errors out. The suggested options to instead overwrite it assume that there was an attempt to install just python@3.9 and I couldn't get it to work for this dependency use-case.

==> Pouring python@3.9-3.9.1_1.catalina.bottle.tar.gz
Error: The `brew link` step did not complete successfully
The formula built, but is not symlinked into /usr/local
Could not symlink bin/2to3
Target /usr/local/bin/2to3
already exists. You may want to remove it:
  rm '/usr/local/bin/2to3'

To force the link and overwrite all conflicting files:
  brew link --overwrite python@3.9

To list all files that would be deleted:
  brew link --overwrite --dry-run python@3.9

Possible conflicting files are:
/usr/local/bin/2to3 -> /Library/Frameworks/Python.framework/Versions/2.7/bin/2to3

Unlinking the pre-installed python@3.9 package and relinking it somehow, magically, fixes this. The brew upgrade is just to make sure that it's not python that's failing anymore.

ALSA check (randomly, in either of the Rt libs) frequently fails on Nixpkgs CI, possibly due to running the checks in
parallel within the main build step? Manually invoking the checks with only one thread.
@rerrahkr
Copy link
Member

LGTM. Would you mind me merging this PR?
After merging, I will try to resolve the conflict with master in #288.

@OPNA2608
Copy link
Member Author

Go ahead. 👍

@rerrahkr rerrahkr merged commit ebc1315 into BambooTracker:link_rtaudio_rtmidi Dec 26, 2020
@rerrahkr
Copy link
Member

Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants