Skip to content

Conversation

@kwvg
Copy link
Collaborator

@kwvg kwvg commented Jul 11, 2024

Additional Information

Despite builders for BSD-based platforms being backported as early as dash#5362, they didn't work on account of using a curl flag, --retry, that was lopped off other builders in dash#3003 as a way to mitigate aberrant Travis CI-specific behaviour.

As the variable $(DOWNLOAD_RETRIES) was removed as part of that mitigation, the introduction of builders that rely on this variable caused failures as the flag was accompanied by nothing. As our CI host isn't based on FreeBSD, this went unnoticed. When deciding between extending the existing patch and reverting it, reverting it proved to be more attractive on account of us no longer using Travis CI and the revert bringing us closer to upstream.

Additionally, the std::unary_function patch that was introduced in dash#5610 proves to be necessary on BSD-based platforms as well (had to be extended for and tested on GhostBSD, based on FreeBSD 13.2-STABLE, Clang 16). Instead of conditionally patching based on platform and compiler, a more reliable patch would be to downgrade the C++ version used to build Boost at the last version to have std::unary_function, which would be C++11, albeit deprecated (source).

Though it's likely this route wasn't taken originally due to compiler errors that happened despite downgrading to C++11. These errors were due to the compiler objecting to std::unary_function usage in Boost headers, despite the backport of bitcoin#25436, which should've solved this problem. The reason the errors were still persisting is because the necessary flag, -DBOOST_NO_CXX98_FUNCTION_BASE, was only applied if --enable-suppress-external-warnings was set. CI didn't catch this, as the flag is always set, to keep log lengths manageable. This has been rectified.

All changes combined, one should be able to build non-Qt Dash binaries using depends though building the Qt client from depends unfortunately remains a problem, even upstream (source, source).

Demo

Based on bbc9957

Built using:

  • depends flags: NO_QT=1 ALLOW_HOST_PACKAGES=1 HOST=x86_64-unknown-freebsd13.2
  • configure flags : --prefix=$(pwd)/depends/x86_64-unknown-freebsd13.2 --enable-debug --enable-suppress-external-warnings --with-gui
  • Qt installed using pkg (qt5)

Dash-Qt running on GhostBSD

Checklist:

  • I have performed a self-review of my own code (note: N/A)
  • I have commented my code, particularly in hard-to-understand areas (note: N/A)
  • I have added or updated relevant unit/integration/functional/e2e tests (note: N/A)
  • I have made corresponding changes to the documentation (note: N/A)
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

kwvg added 3 commits July 11, 2024 16:31
…when Travis has network issues)

commits reverted:
- 9a45241
- 41f46f5
… on `--enable-suppress-external-warnings`

This is in line with how it's done in 880d4aa upstream. This wasn't
noticed with CI as we build with `--enable-suppress-external-warnings`
by default.
@kwvg kwvg added this to the 21.1 milestone Jul 11, 2024
…th c++11

we can revert this after upgrading to boost 1.81
@kwvg kwvg marked this pull request as ready for review July 11, 2024 19:22
@kwvg kwvg requested review from PastaPastaPasta, UdjinM6 and knst July 11, 2024 19:22
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

Compiled with no issues on macos 14.5, Apple clang version 15.0.0 (clang-1500.3.9.4)

light ACK bbc9957

endif
$(package)_config_libraries=filesystem,test
$(package)_cxxflags=-std=c++17
$(package)_cxxflags=-std=c++11
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you add TODO here so far as it is not permanent change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

I meant TODO right here, so, next release we can re-evaluate it if it is still necessary or can be removed, but I guess you should not forget it yourself :D

@kwvg kwvg requested a review from knst July 15, 2024 19:00
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK bbc9957

Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

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

ACK bbc9957

got a build succeed with gcc-13.2.0-4ubuntu3 on ubuntu (not freebds though)

@PastaPastaPasta PastaPastaPasta merged commit debc706 into dashpay:develop Jul 19, 2024
@UdjinM6 UdjinM6 modified the milestones: 21.1, 21.2 Aug 8, 2024
@UdjinM6 UdjinM6 modified the milestones: 21.2, 22 Oct 29, 2024
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.

4 participants