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 OpenBSD build #2080

Merged
merged 20 commits into from
Nov 22, 2024
Merged

Fix OpenBSD build #2080

merged 20 commits into from
Nov 22, 2024

Conversation

g0mb4
Copy link
Collaborator

@g0mb4 g0mb4 commented Nov 15, 2024

Using OpenBSD 7.6.

Try to close #2073, unfortunately #2075 is not enough.

Copy link

netlify bot commented Nov 15, 2024

Deploy Preview for conkyweb canceled.

Name Link
🔨 Latest commit fc7dead
🔍 Latest deploy log https://app.netlify.com/sites/conkyweb/deploys/674113562ec2b40008de05eb

@github-actions github-actions bot added sources PR modifies project sources 3rdparty suggests changes to 3rd party dependencies power related to battery and power management code labels Nov 15, 2024
@g0mb4 g0mb4 changed the title [WIP] Fixing OpenBSD build. [WIP] Fix OpenBSD build. Nov 15, 2024
Work in progress.
@Caellian Caellian added bug related to incorrect existing implementation of some functionality os: openbsd related to OpenBSD labels Nov 15, 2024
@Caellian

This comment was marked as outdated.

Signed-off-by: Tin Švagelj <[email protected]>
@Caellian
Copy link
Collaborator

Referencing #2052 as cause of issues.

@g0mb4
Copy link
Collaborator Author

g0mb4 commented Nov 15, 2024

I'm just running make and fixing the warnings/errors :)

I want it to compile first, then fix the issues with the other platforms.

@g0mb4
Copy link
Collaborator Author

g0mb4 commented Nov 15, 2024

And yes, void f(void) is done by my autopiloted brain.

@Caellian Caellian force-pushed the fix/openbsd76-build branch from 13e3858 to 150c5dd Compare November 15, 2024 22:09
- Fix `~` substitution.
- Add more tests for to_real_path.

Signed-off-by: Tin Švagelj <[email protected]>
@github-actions github-actions bot added the tests related to project tests label Nov 15, 2024
@Caellian Caellian force-pushed the fix/openbsd76-build branch from 3915c89 to a64f0af Compare November 15, 2024 22:28
Signed-off-by: Tin Švagelj <[email protected]>
@github-actions github-actions bot added the gh-actions suggest changing GitHub actions label Nov 15, 2024
@Caellian Caellian force-pushed the fix/openbsd76-build branch from f1aad1b to 2255929 Compare November 15, 2024 23:05
Signed-off-by: Tin Švagelj <[email protected]>
This reverts commit b8c7012.

None of the workers work "out of the box". Better to put this into a
separate PR to avoid blocking the more important one.
@github-actions github-actions bot removed the gh-actions suggest changing GitHub actions label Nov 15, 2024
@Caellian Caellian force-pushed the fix/openbsd76-build branch from fefa25a to 7fc6c69 Compare November 16, 2024 03:32
Signed-off-by: Tin Švagelj <[email protected]>
@Caellian Caellian force-pushed the fix/openbsd76-build branch 3 times, most recently from 8b795e0 to 905aa15 Compare November 16, 2024 08:21
Signed-off-by: Tin Švagelj <[email protected]>
Per documentation:
<XXX>_LIBRARIES - only the libraries (without the '-l')
<XXX>_LINK_LIBRARIES - the libraries and their absolute paths

Using _LIBRARIES assumes that appropriate default -L arguments will be
provided by the environment/system. CMake discourages directly adding
paths with -L or link_directiories because it polutes the environment
and can cause issues. So the only way to ensure correct build on some
platforms is to use absolute paths provided by _LINK_LIBRARIES.

Signed-off-by: Tin Švagelj <[email protected]>
@Caellian Caellian force-pushed the fix/openbsd76-build branch from a0a17fb to 96e84fa Compare November 16, 2024 11:37
It's default so it only causes compile warnings

Signed-off-by: Tin Švagelj <[email protected]>
@Caellian
Copy link
Collaborator

I believe that pretty much covers my intent behind #2075. I'm building this from a minimal VM environment and would appreciate if you could test it @g0mb4. Feel free to implement any of the added stubs before requesting review.

For future reference (CI?), package names:

Minimal package dependencies:

  • git (to clone)
  • cmake
  • gperf
  • lua-5.3
  • X11 (comes preinstalled)
    • imlib2

Optional pkg deps:

  • cairo (if BUILD_LUA_CAIRO, BUILD_LUA_CAIRO_XLIB)
  • imlib2 (if BUILD_LUA_IMLIB2)
  • librsvg (if BUILD_LUA_RSVG)

@g0mb4
Copy link
Collaborator Author

g0mb4 commented Nov 16, 2024

Wow! Thank you for the help! I'll test is as soon as possible.

g0mb4 added a commit to g0mb4/conky that referenced this pull request Nov 16, 2024
A cross-platform solution is already implemented in brndnmtthws#2080.
This PR will be rebased *after* the OpenBSD fix was merged.
_LINK_LIBRARIES is used so there will be no more -lX11 args.

Signed-off-by: Tin Švagelj <[email protected]>
@g0mb4
Copy link
Collaborator Author

g0mb4 commented Nov 18, 2024

It works on my setup.

@g0mb4 g0mb4 marked this pull request as ready for review November 18, 2024 08:24
@g0mb4 g0mb4 changed the title [WIP] Fix OpenBSD build. Fix OpenBSD build. Nov 18, 2024
@g0mb4
Copy link
Collaborator Author

g0mb4 commented Nov 18, 2024

I think adding more features really should be part of a new PR.

@Caellian Caellian changed the title Fix OpenBSD build. Fix OpenBSD build Nov 18, 2024
@Caellian
Copy link
Collaborator

Caellian commented Nov 19, 2024

Alright, then it's done.

I didn't test all possible features because built time was atrocious, but I think they're likely to work. Core, X11 stuff and LUA stuff is confirmed to build on different architectures.

Following BUILD_* features might warrant additional attention in separate PRs, but in general they should work if appropriate dependencies are installed:

BUILD_* Works Port (/dependency)
AUDACIOUS should audacious
CURL should curl (in default install)
HTTP should libmicrohttpd
ICAL should libical
INTEL_BACKLIGHT unknown /sys/class/backlight/intel_backlight
IRC should libircclient
JOURNAL unlikely systemd
MYSQL should mysql.h provider (mariadb?)
NVIDIA nvidia support flaky NVCtrl header provider
PULSEAUDIO should pulseaudio
RSS should curl (default), libxml-2.13
WLAN not used; always enabled - packagers should enable it by default to avoid breakage ioctl and native headers
XMMS2 should xmms2

Adding CI would be nice even if OpenBSD isn't a primary support target because it would improve code quality by failing on non standard language/stdlib use.

@Caellian Caellian requested a review from brndnmtthws November 19, 2024 00:10
src/common.cc Outdated Show resolved Hide resolved
@g0mb4 g0mb4 mentioned this pull request Nov 22, 2024
@Caellian Caellian force-pushed the fix/openbsd76-build branch 2 times, most recently from 0fab7f4 to c0f6a3c Compare November 22, 2024 23:22
This avoids feature regression but still works better for OpenBSD than
always returning an empty string when to_real_path is called.

Signed-off-by: Tin Švagelj <[email protected]>
@Caellian Caellian force-pushed the fix/openbsd76-build branch from c0f6a3c to fc7dead Compare November 22, 2024 23:27
Copy link
Collaborator

@Caellian Caellian left a comment

Choose a reason for hiding this comment

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

From perspective of already supported platforms these are minor consistency tweaks.

From perspective of OpenBSD, it allows conky to at least build which is most important because official OpenBSD port is stuck on a very old version due to build issues.

I'm merging this so @g0mb4 can continue working on NetBSD support in #2089 without having to deal with rebasing.

@brndnmtthws feel free to revert this PR if you don't like something about this, but I'll merge it for now given that CI passes (builds) so none of the already supported platforms should be affected. See the commit message for an exhaustive summary.

@g0mb4 don't delete the branch in case @brndnmtthws rolls it back and requests edits.

@Caellian Caellian merged commit 47ec9f9 into brndnmtthws:main Nov 22, 2024
39 checks passed
brndnmtthws pushed a commit that referenced this pull request Nov 29, 2024
* Implement `-U` for Haiku.

This patch also fixes a compilation error on Haiku.

Compiled with `cmake -DBUILD_X11=FALSE ..`

Tested on Haiku-r1beta5.
Takes part of #2072.

* Revert `common.cc`.

A cross-platform solution is already implemented in #2080.
This PR will be rebased *after* the OpenBSD fix was merged.

* Haiku: Remove `wordexp.h`.

Use the `to_real_path()` used for OpenBSD.

* Haiku: Remove `HAIKU_HOME_DIR`.

It was not used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rdparty suggests changes to 3rd party dependencies audio related to audacious, pulseaudio, and other audio components bug related to incorrect existing implementation of some functionality build system related to build system (CMake) and/or building process/assumptions dependencies adds or removes dependencies, or suggests alternatives os: openbsd related to OpenBSD power related to battery and power management code sources PR modifies project sources tests related to project tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: unable to build due to Vc on OpenBSD 7.6
2 participants