Page MenuHomeFreeBSD
Differential D39978

graphics/spectacle: make pipewire optional
ClosedPublic
Actions

Authored by vvd on May 5 2023, 1:06 PM.
Tags
Referenced Files
Unknown Object (File)
Sun, May 4, 4:14 AM
Unknown Object (File)
Mon, Apr 21, 10:54 AM
Unknown Object (File)
Sun, Apr 20, 12:23 AM
Unknown Object (File)
Sat, Apr 19, 1:44 AM
Unknown Object (File)
Fri, Apr 18, 1:21 AM
Unknown Object (File)
Wed, Apr 16, 7:33 AM
Unknown Object (File)
Mon, Apr 14, 9:05 AM
Unknown Object (File)
Sat, Apr 12, 8:38 PM
View All Files
Subscribers
arrowd

Details

Reviewers
tcberner
arrowd
Group Reviewers
kde
Commits
R11:68e029f386b5: graphics/spectacle: make pipewire optional
Summary

Tested on 13.2 amd64.
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=270982

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

vvd requested review of this revision.May 5 2023, 1:06 PM
vvd created this revision.
arrowd added a subscriber: arrowd.May 6 2023, 8:51 AM

This diff doesn't apply with arc too. Can you please re-generate it using Arcanist?

vvd updated this revision to Diff 121601.May 6 2023, 10:27 AM

I think it was copy&paste from console issue - tabs replaced with spaces and etc.
"git diff > spectacle.diff" and uploaded file spectacle.diff now.

arrowd added inline comments.May 11 2023, 6:23 AM
graphics/spectacle/files/extra-patch-pipewire-off
11

Double negation looks weird. Can you rename the CMake variable to ENABLE_PIPEWIRE?

32
arrowd added inline comments.May 11 2023, 6:26 AM
graphics/spectacle/files/extra-patch-pipewire-off
11

Ah, nevermind, krfb already has a variable that is named DISABLE_*.

vvd added a reviewer: arrowd.May 11 2023, 10:06 AM
vvd added a comment.EditedMay 11 2023, 10:26 AM

BTW, using spectacle with this patch for 3 weeks - screenshots work fine.

arrowd added a comment.May 11 2023, 11:34 AM

Address another my comment and it should be fine.

vvd updated this revision to Diff 121853.May 11 2023, 12:41 PM
arrowd added inline comments.May 12 2023, 11:15 AM
graphics/spectacle/files/extra-patch-pipewire-off
50
72

Where does this definition gets added? CMake doesn't add all its variables as -D flags to the compiler. You probably need to look at some sort of config.h.in and add this variable there.

arrowd added inline comments.May 12 2023, 11:29 AM
graphics/spectacle/files/extra-patch-pipewire-off
13

Actually, this check isn't needed at all. If KPipeWire wasn't found, CMake will stop due to REQUIRED keyword`. If it didn't then no need to check for KPipeWire_FOUND.

vvd updated this revision to Diff 121995.May 15 2023, 10:01 PM
vvd added inline comments.
graphics/spectacle/files/extra-patch-pipewire-off
11

Yes, it was made like in krfb.

32

Do I need to update the patch?

50

What about line #30?
Same "if(PIPEWIRE_FOUND)".

72

Maybe this one "set(PIPEWIRE_FOUND 1)"?
And yes - it's work as expected.
I'm using it for 3 weeks already.

arrowd added inline comments.May 16 2023, 5:02 AM
graphics/spectacle/files/extra-patch-pipewire-off
12

You don't need PIPEWIRE_FOUND at all, just use NOT DISABLE_PIPEWIRE.

vvd updated this revision to Diff 121997.May 16 2023, 10:15 AM

Added #cmakedefine PIPEWIRE_FOUND 1 to pass on option PIPEWIRE_FOUND from cmake to sources.

arrowd accepted this revision.May 16 2023, 3:20 PM
This revision is now accepted and ready to land.May 16 2023, 3:20 PM
Closed by commit R11:68e029f386b5: graphics/spectacle: make pipewire optional (authored by vvd). · Explain WhyMay 16 2023, 3:41 PM
This revision was automatically updated to reflect the committed changes.
vvd added a commit: R11:68e029f386b5: graphics/spectacle: make pipewire optional.

Revision Contents
Changeset List

PathSize
graphics/
spectacle/
9 lines
files/
94 lines

Diff 121997

View Options

graphics/spectacle/Makefile

Loading...
View Options

graphics/spectacle/files/extra-patch-pipewire-off

Loading...