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

Fix build issues and more on FreeBSD #4111

Closed
wants to merge 2,351 commits into from
Closed

Fix build issues and more on FreeBSD #4111

wants to merge 2,351 commits into from

Conversation

alonsobsd
Copy link

mixxx was update to 2.3 on FreeBSD but I applied some patches to fix some issues

Holzhaus and others added 30 commits June 21, 2021 21:26
Add lost timelinePath and logFlushLevel parsing
QML: Add support for selecting/loading effects
…ware/mixxxdj/mixxx2-4/. Compile QM files out of TS files that are used by the localized app
Enable export of hot cue colours to EnginePrime
Update Mixxx version in LICENSE to 2.4
Classes inheriting from QAbstractItemModel are QObjects, but also have a
`parent()` method that takes a QModelIndex. In these cases, an error
like this may be thrown:

    src/util/parented_ptr.h: In instantiation of ‘parented_ptr<T>::~parented_ptr() [with T = TreeItemModel]’:
    src/library/itunes/itunesfeature.cpp:67:55:   required from here
    src/util/parented_ptr.h:31:45: error: no matching function for call to ‘TreeItemModel::parent()’
       31 |         DEBUG_ASSERT(!m_ptr || m_ptr->parent());
          |                                ~~~~~~~~~~~~~^~

This fixes the issue by casting to QObject first.
LibraryFeature: Rename getChildModel to sidebarModel and make it const
ronso0 and others added 12 commits July 12, 2021 19:46
mixxxlibraryfeature: remove wrong ENGINE_PRIME ifdef
We currently have two beats implementations (`BeatGrid` and `BeatMap`)
and both support scaling and translating. Hence, it's unnecessary to
maintain these flags. In fact, the `BEATSCAP_SCALE` capability wasn't
even checked in the calling code but nobody noticed because both support
it anyway.

The `BEATSCAP_ADDREMOVE` and `BEATSCAP_MOVEBEAT` capabilities are only
supported by the `BeatMap`, but we don't have any code that makes use of
them. Therefore, these can be removed as well.
CMake: use CXX_COMPILER_LAUNCHER instead of RULE_LAUNCH_COMPILE
The previous implementation was hard to use on a touchscreen
because the surface where it was touching was very small.
The new version makes use of a slider internally which allows
the control to be changed more intiutively by swiping.
…iendlier

QMLDemo: improve OrientationToggleButton touch friendlyness
@github-actions github-actions bot added the build label Jul 17, 2021
@Be-ing Be-ing changed the base branch from main to 2.3 July 17, 2021 18:39
@Be-ing
Copy link
Contributor

Be-ing commented Jul 17, 2021

Thanks for this! Can you do an interactive rebase on the 2.3 branch and make the commit messages more descriptive?

Where does the __FREEBSD__ define come from? Our CMake scripts don't define it. Is that always defined when compiling on FreeBSD?

@@ -77,6 +77,8 @@ QString VersionStore::platform() {
QString base = QStringLiteral("Linux");
#elif defined(__WINDOWS__)
QString base = QStringLiteral("Windows");
#elif defined(__FreeBSD__)
QString base = QStringLiteral("FreeBSD");
Copy link
Contributor

Choose a reason for hiding this comment

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

extra spaces at the end of the line

@Be-ing
Copy link
Contributor

Be-ing commented Jul 17, 2021

Maybe we could use #ifdef __UNIX__ instead to make this more general?

@Be-ing
Copy link
Contributor

Be-ing commented Jul 17, 2021

Let's use __BSD__ instead. We already have that set by the build system:

elseif(UNIX)
  if(APPLE)
    target_compile_definitions(mixxx-lib PUBLIC __APPLE__)
  else()
    target_compile_definitions(mixxx-lib PUBLIC __UNIX__)
    if(CMAKE_SYSTEM_NAME STREQUAL Linux)
      target_compile_definitions(mixxx-lib PUBLIC __LINUX__)
    elseif(CMAKE_SYSTEM_NAME MATCHES "^.*BSD$")
      target_compile_definitions(mixxx-lib PUBLIC __BSD__)
    endif()
  endif()
endif()

Comment on lines +60 to +64
if(${CMAKE_SYSTEM_NAME} MATCHES "FreeBSD")
NAMES usb
else
NAMES usb-1.0
endif()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should work without any special casing for BSD:

  NAMES usb-1.0 usb

Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

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

The proposed changes have to be applied upstream. We are not going to maintain custom patches for 3rd party libs.

@Be-ing
Copy link
Contributor

Be-ing commented Jul 17, 2021

@uklotzde I do not understand your comment. This does not change third party dependencies.

@Be-ing
Copy link
Contributor

Be-ing commented Jul 17, 2021

Oh do you mean the hack for SPSCQue? Upstream already decided they didn't want that so we decided to keep it in Mixxx.

@uklotzde
Copy link
Contributor

I have noticed changes in lib for SPSC and Kaitai Struct. If you need to apply patches for FreeBSD you have to do this during packaging. Otherwise please submit the changes upstream.

The PR is not reviewable until it is properly rebased.

@uklotzde
Copy link
Contributor

If it only modifies a custom patch of us it is acceptable. I didn't verify this.

@uklotzde
Copy link
Contributor

Disclaimer: I am not able to properly review PRs during the next days. I'll try to check occasionally for updates.

@Be-ing
Copy link
Contributor

Be-ing commented Jul 17, 2021

The change to Kaitai Struct should be sent upstream.

@Be-ing
Copy link
Contributor

Be-ing commented Jul 17, 2021

I would prefer the patch for Kaitai Struct to stay in the FreeBSD port until that's merged upstream.

@Be-ing
Copy link
Contributor

Be-ing commented Jul 20, 2021

I redid these changes except the one for the Kaitai Struct file on the 2.3 branch in #4122, #4123, and #4124. Please submit the fix for Kaitai Struct upstream. In the meantime you can keep using that patch in the FreeBSD port. Thanks for bringing these issues to our attention.

@Be-ing Be-ing closed this Jul 20, 2021
@alonsobsd
Copy link
Author

Thanks for you too

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

Successfully merging this pull request may close these issues.