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.

Sign up for GitHub

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

Jump to bottom

depends: systemtap 4.8 #26945

Merged
merged 2 commits into from
Jan 26, 2023
Merged

depends: systemtap 4.8 #26945

merged 2 commits into from
Jan 26, 2023

Conversation

fanquake
Copy link
Member

@fanquake fanquake commented Jan 22, 2023 •
edited
Loading

Updates systemtap to 4.8.
Includes acc2895 from #25972.
Will half (depends) fix #26916.

Release notes etc: https://lwn.net/Articles/913908/.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 22, 2023 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK hebasto, 0xB10C

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #25972 (build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set by fanquake)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@DrahtBot DrahtBot mentioned this pull request Jan 22, 2023
Merged
@fanquake fanquake marked this pull request as ready for review January 23, 2023 10:10
@fanquake fanquake requested a review from 0xB10C January 23, 2023 10:10
@0xB10C
Copy link
Contributor

0xB10C commented Jan 23, 2023

ACK a913793

Changes to the depends look good. Hash matches with what I get.

Release notes etc: https://lwn.net/Articles/913908/.

sourceware.org/PR29246 is relevant to us which is the only commit changing the includes/sys/sdt.h file. But it shouldn't affect our use of sdt.h.

I've previously checked that removing the problematic variadic in a patch does not cause any problems for us as we don't use it. This also doesn't conflict with #26593 as we use a different variadic there.

@hebasto
Copy link
Member

hebasto commented Jan 23, 2023 •
edited
Loading

Updates systemtap to 4.8.
Includes acc2895 from #25972.
Will half (depends) fix #26916.

Not able to reproduce #26916 when building with depends on the master branch on Ubuntu 22.10:

$ make -C depends NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_NATPMP=1
$ ./autogen.sh
$ ./configure CONFIG_SITE=$PWD/depends/x86_64-pc-linux-gnu/share/config.site CC=clang CXX=clang++
$ make clean
$ make
$ clang --version
Ubuntu clang version 15.0.2-1
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

Is it a result of bumping to 4.8?

@fanquake
Copy link
Member Author

Is it a result of bumping

No it's probably because you don't have any warnings turned on.

@hebasto
Copy link
Member

hebasto commented Jan 23, 2023

Is it a result of bumping

No it's probably because you don't have any warnings turned on.

Right... 🤦‍♂️

hebasto
hebasto approved these changes Jan 23, 2023
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK a913793, tested on Ubuntu 22.10.

depends/patches/systemtap/fix_variadic_warning.patch Outdated Show resolved Hide resolved
depends/patches/systemtap/fix_variadic_warning.patch Show resolved Hide resolved
hebasto
hebasto approved these changes Jan 23, 2023
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

re-ACK df7ae8b.

@0xB10C
Copy link
Contributor

0xB10C commented Jan 24, 2023

ACK df7ae8b

@fanquake fanquake merged commit eee2c28 into bitcoin:master Jan 26, 2023
@fanquake fanquake deleted the systemtap_4_8 branch January 26, 2023 13:38
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 26, 2023
@fanquake
Merge bitcoin#26945: depends: systemtap 4.8
5356462 
df7ae8b depends: systemtap: remove variadic params that trigger compiler warnings (Cory Fields)
a66d82e depends: systemtap 4.8 (fanquake)

Pull request description:

  Updates systemtap to 4.8.
  Includes acc2895 from bitcoin#25972.
  Will half (depends) fix bitcoin#26916.

  Release notes etc: https://lwn.net/Articles/913908/.

ACKs for top commit:
  0xB10C:
    ACK df7ae8b
  hebasto:
    re-ACK df7ae8b.

Tree-SHA512: 0f64fba87888058380183f38b6ace7b71f4a8b5503eb32b82b283a40c99d92c449c60deecc2386df5084235cfd760af6c1e7f432fa1bd30f97bb42f100f23d62
@bitcoin bitcoin locked and limited conversation to collaborators Jan 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hebasto hebasto hebasto approved these changes

@0xB10C 0xB10C Awaiting requested review from 0xB10C

Assignees
No one assigned
Labels
Build system
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

tracepoints: gnu-zero-variadic-macro-arguments warnings
5 participants
@fanquake @DrahtBot @0xB10C @hebasto @theuni