ci: fixes macOS builds #1019

mrpollo · 2024-11-12T18:56:18Z

This pr fixes the macOS CI

farhangnaderi · 2024-11-12T19:13:13Z

This pr fixes the macOS CI

Thanks Ramon!

facontidavide · 2024-11-12T19:28:11Z

Thanks, but all the other builds are failing (see CI)

farhangnaderi · 2024-11-12T19:45:14Z

seems the touch ups with protobuf are failing. I will take a look.

farhangnaderi · 2024-11-18T05:35:06Z

Thanks, but all the other builds are failing (see CI)

It should be ok now.
Edit: Our local Actions passed but not here.

mrpollo · 2024-11-18T07:22:09Z

COMPILE.md

@@ -118,7 +120,7 @@ export LDFLAGS="$QT_HOME/lib"
 Clone the repository into **~/plotjuggler_ws**:

 ```shell
-git clone https://github.com/facontidavide/PlotJuggler.git ~/plotjuggler_ws/src/PlotJuggler
+git clone https://github.com/PX4/PlotJuggler.git ~/plotjuggler_ws/src/PlotJuggler


Please revert

exactly 😄

My bad! Done.

mrpollo · 2024-11-18T17:15:28Z

plotjuggler_plugins/DataStreamZMQ/CMakeLists.txt

@@ -10,24 +10,24 @@ endif()
 find_package(ZeroMQ QUIET)

 if(ZeroMQ_FOUND)
-    message(STATUS "[ZeroMQ] found")
+    # message(STATUS "[ZeroMQ] found")


can you please cleanup comments?

what is going on here?

Are you intentionally removing ZeroMQ plugin?

I was. Seems ZMQ library is not found sometimes on Mac. I think what I added would tolerate it and pass the build if it cannot find it.

mrpollo · 2024-11-18T17:34:45Z

@facontidavide we made a few changes can you please approve the workflow runs? thank you

farhangnaderi · 2024-12-19T00:39:25Z

I rebased and seems on local repo it passed all CI checks!

facontidavide

see comments from @mrpollo

RicardoM17 · 2025-01-07T13:30:57Z

I have made a PR with the requested changes from the review of this PR. All should be ok but please confirm. If so feel free to merge that one and close this one, or cherry-pick that commit here (I couldn't push to this branch directly). From my side there is no preference.

Hope that is ok @farhangnaderi . Thanks for the fix also @mrpollo

farhangnaderi · 2025-01-07T13:59:15Z

I have made a PR with the requested changes from the review of this PR. All should be ok but please confirm. If so feel free to merge that one and close this one, or cherry-pick that commit here (I couldn't push to this branch directly). From my side there is no preference.

Hope that is ok @farhangnaderi . Thanks for the fix also @mrpollo

Thanks @RicardoM17 . I am not sure how this PR would be different. The issue it makes is zmq not found if we want to have it built.

farhangnaderi · 2025-01-08T14:19:12Z

I have made a PR with the requested changes from the review of this PR. All should be ok but please confirm. If so feel free to merge that one and close this one, or cherry-pick that commit here (I couldn't push to this branch directly). From my side there is no preference.
Hope that is ok @farhangnaderi . Thanks for the fix also @mrpollo

Thanks @RicardoM17 . I am not sure how this PR would be different. The issue it makes is zmq not found if we want to have it built.

This is the issue I am talking about, that is why I had suppressed zmq.

[ 85%] Automatic MOC and UIC for target DataStreamZMQ
[ 85%] Built target DataStreamZMQ_autogen
[ 85%] Generating ui_datastream_zmq.h
[ 86%] Building CXX object plotjuggler_plugins/DataStreamZMQ/CMakeFiles/DataStreamZMQ.dir/DataStreamZMQ_autogen/mocs_compilation.cpp.o
In file included from /Users/farhang/plotjuggler_ws/build/PlotJuggler/plotjuggler_plugins/DataStreamZMQ/DataStreamZMQ_autogen/mocs_compilation.cpp:2:
In file included from /Users/farhang/plotjuggler_ws/build/PlotJuggler/plotjuggler_plugins/DataStreamZMQ/DataStreamZMQ_autogen/EWIEGA46WW/moc_datastream_zmq.cpp:10:
In file included from /Users/farhang/plotjuggler_ws/build/PlotJuggler/plotjuggler_plugins/DataStreamZMQ/DataStreamZMQ_autogen/EWIEGA46WW/../../../../../../src/PlotJuggler/plotjuggler_plugins/DataStreamZMQ/datastream_zmq.h:9:
/Users/farhang/plotjuggler_ws/build/PlotJuggler/plotjuggler_plugins/DataStreamZMQ/DataStreamZMQ_autogen/EWIEGA46WW/../../../../../../src/PlotJuggler/plotjuggler_plugins/DataStreamZMQ/zmq.hpp:36:10: fatal error: 'zmq.h' file not found
   36 | #include <zmq.h>
      |          ^~~~~~~
1 error generated.
make[2]: *** [plotjuggler_plugins/DataStreamZMQ/CMakeFiles/DataStreamZMQ.dir/DataStreamZMQ_autogen/mocs_compilation.cpp.o] Error 1
make[1]: *** [plotjuggler_plugins/DataStreamZMQ/CMakeFiles/DataStreamZMQ.dir/all] Error 2

Pr fix ci (#3) ci: Fixes macOS builds (#1) adds runners for both Intel and Apple silicon macs --------- Co-authored-by: Daniel Agar <[email protected]> Co-authored-by: Ramon Roche <[email protected]>

farhangnaderi · 2025-01-13T06:27:48Z

@mrpollo @facontidavide You may double check.
Thanks,

mrpollo

we are almost there, I left a few comments please review

mrpollo · 2025-01-13T17:27:16Z

.github/workflows/macos.yaml

-          key: ${{ runner.os }}-QtCache
+      - name: Prepare Homebrew and Install Dependencies
+        run: |
+          rm -rf /usr/local/var/homebrew/locks


is this necessary? seems like it goes against homebrew policy

mrpollo · 2025-01-13T17:28:04Z

.github/workflows/ubuntu.yaml

@@ -3,7 +3,6 @@ name: ubuntu
 on: [push, pull_request]

 jobs:
-
  ubuntu-build:


can you please revert the change to this file?

mrpollo · 2025-01-13T17:28:57Z

installer/io.plotjuggler.application/meta/installscript.qs

@@ -101,4 +101,4 @@ Component.prototype.componentSelectionPageEntered = function()
    if (installer.fileExists(dir) && installer.fileExists(dir + "/maintenancetool.exe")) {
        installer.execute(dir + "/maintenancetool.exe", ["purge", "-c"]);
    }
-}


can you also make sure this file isn't touched by this pr?

mrpollo · 2025-01-13T17:29:06Z

installer/io.plotjuggler.application/meta/targetwidget.ui

@@ -110,4 +110,4 @@
 </widget>
 <resources/>
 <connections/>
-</ui>


same here please

farhangnaderi mentioned this pull request Nov 18, 2024

Fixing CI for upstream 1019 PX4/PlotJuggler#2

Merged

mrpollo commented Nov 18, 2024

View reviewed changes

mrpollo force-pushed the main branch 5 times, most recently from ed79ea6 to dc9cb8c Compare November 18, 2024 17:32

mrpollo force-pushed the main branch from dc9cb8c to 7657fb5 Compare November 18, 2024 17:43

farhangnaderi force-pushed the main branch 3 times, most recently from 085c880 to b1ad089 Compare December 18, 2024 23:23

facontidavide requested changes Dec 19, 2024

View reviewed changes

RicardoM17 mentioned this pull request Jan 7, 2025

ci: fixes macOS builds #1033

Open

Tolerating no ZMQ, protobuf version (#4)

6fd39d2

Pr fix ci (#3) ci: Fixes macOS builds (#1) adds runners for both Intel and Apple silicon macs --------- Co-authored-by: Daniel Agar <[email protected]> Co-authored-by: Ramon Roche <[email protected]>

farhangnaderi force-pushed the main branch 2 times, most recently from e71e45c to 6fd39d2 Compare January 13, 2025 06:19

mrpollo commented Jan 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: fixes macOS builds #1019

ci: fixes macOS builds #1019

mrpollo commented Nov 12, 2024

farhangnaderi commented Nov 12, 2024

facontidavide commented Nov 12, 2024

farhangnaderi commented Nov 12, 2024

farhangnaderi commented Nov 18, 2024 •

edited

Loading

mrpollo Nov 18, 2024

facontidavide Dec 19, 2024

farhangnaderi Jan 13, 2025

mrpollo Nov 18, 2024

facontidavide Dec 19, 2024

farhangnaderi Jan 13, 2025

mrpollo commented Nov 18, 2024

farhangnaderi commented Dec 19, 2024 •

edited

Loading

facontidavide left a comment

RicardoM17 commented Jan 7, 2025

farhangnaderi commented Jan 7, 2025

farhangnaderi commented Jan 8, 2025

farhangnaderi commented Jan 13, 2025

mrpollo left a comment

mrpollo Jan 13, 2025

mrpollo Jan 13, 2025

mrpollo Jan 13, 2025

mrpollo Jan 13, 2025

ci: fixes macOS builds #1019

Are you sure you want to change the base?

ci: fixes macOS builds #1019

Conversation

mrpollo commented Nov 12, 2024

farhangnaderi commented Nov 12, 2024

facontidavide commented Nov 12, 2024

farhangnaderi commented Nov 12, 2024

farhangnaderi commented Nov 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrpollo commented Nov 18, 2024

farhangnaderi commented Dec 19, 2024 • edited Loading

facontidavide left a comment

Choose a reason for hiding this comment

RicardoM17 commented Jan 7, 2025

farhangnaderi commented Jan 7, 2025

farhangnaderi commented Jan 8, 2025

farhangnaderi commented Jan 13, 2025

mrpollo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

farhangnaderi commented Nov 18, 2024 •

edited

Loading

farhangnaderi commented Dec 19, 2024 •

edited

Loading