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

mosquitto: 2.0.18 -> 2.0.20 #349233

Merged
merged 1 commit into from
Oct 23, 2024
Merged

mosquitto: 2.0.18 -> 2.0.20 #349233

merged 1 commit into from
Oct 23, 2024

Conversation

datafoo
Copy link
Contributor

@datafoo datafoo commented Oct 17, 2024 •
edited
Loading

https://github.com/eclipse/mosquitto/blob/v2.0.20/ChangeLog.txt

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@mweinelt @SuperSandro2000

Add a 👍 reaction to pull requests you find important.

@datafoo datafoo added the backport release-24.05 Backport PR automatically label Oct 17, 2024
@kirillrdy
Copy link
Member

@datafoo how about this as url to change log https://github.com/eclipse/mosquitto/blob/v2.0.20/ChangeLog.txt ?

@datafoo datafoo force-pushed the mosquitto branch 3 times, most recently from 54f6d64 to e905aed Compare October 17, 2024 09:00
kirillrdy
kirillrdy approved these changes Oct 17, 2024
View reviewed changes
Copy link
Member

@kirillrdy kirillrdy left a comment

Choose a reason for hiding this comment

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

didn't want to increase scope of this PR, but since you've added changlog

@datafoo
Copy link
Contributor Author

datafoo commented Oct 17, 2024

didn't want to increase scope of this PR, but since you've added changlog

It is OK, I learnt something new in the process.

@kirillrdy kirillrdy self-requested a review October 17, 2024 09:46
@kirillrdy
Copy link
Member

kirillrdy commented Oct 17, 2024 •
edited
Loading

@ofborg build mosquitto

@kirillrdy
Copy link
Member

x86_64-darwin build broken https://logs.ofborg.org/?key=nixos/nixpkgs.349233&attempt_id=9d4cea02-01e1-431a-8a17-d5ef34cd45fe

@datafoo
Copy link
Contributor Author

datafoo commented Oct 18, 2024 •
edited
Loading

I do not know what to do with this broken build!

In the meantime, I have extracted the cleanup in its own dedicated PR #349473.

@kirillrdy
Copy link
Member

maybe @NixOS/darwin-maintainers can help

@Aleksanaa
Copy link
Member

This seems to be the same with eclipse-mosquitto/mosquitto#3025. I have no idea what is going wrong with their cmake files, would be good to ask a cmake expert to check it.

@Aleksanaa
Copy link
Member 

Undefined symbols for architecture arm64:
  "_mosquitto_callback_register", referenced from:
      _mosquitto_plugin_init in mosquitto_message_timestamp.c.o
  "_mosquitto_callback_unregister", referenced from:
      _mosquitto_plugin_cleanup in mosquitto_message_timestamp.c.o
  "_mosquitto_property_add_string_pair", referenced from:
      _callback_message in mosquitto_message_timestamp.c.o
ld: symbol(s) not found for architecture arm64
clang-16: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [plugins/message-timestamp/CMakeFiles/mosquitto_message_timestamp.dir/build.make:97: plugins/message-timestamp/mosquitto_message_timestamp.dylib] Error 1
make[1]: *** [CMakeFiles/Makefile2:547: plugins/message-timestamp/CMakeFiles/mosquitto_message_timestamp.dir/all] Error 2

@datafoo
Copy link
Contributor Author

datafoo commented Oct 18, 2024

This seems to be the same with eclipse/mosquitto#3025. I have no idea what is going wrong with their cmake files, would be good to ask a cmake expert to check it.

So, is the problem related to this PR or was it already here before?

@Aleksanaa
Copy link
Member

It was something between 2.0.18 and 2.0.20 with upstream. Nothing wrong on our side.

@datafoo
Copy link
Contributor Author

datafoo commented Oct 18, 2024

The changelog does mention:

Client library:

Aleksanaa
Aleksanaa reviewed Oct 18, 2024
View reviewed changes
pkgs/servers/mqtt/mosquitto/default.nix Outdated

src = fetchFromGitHub {
owner = "eclipse";
repo = "mosquitto";
rev = "v${version}";
hash = "sha256-Vs0blV2IhnlEAm0WtOartz+0vLesJfp78FNJCivRxHk=";
hash = "sha256-oZo6J6mxMC05jJ8RXIunOMB3kptA6FElchKlg4qmuQ8=";
};

patches = lib.optionals stdenv.hostPlatform.isDarwin [
Copy link
Member

Choose a reason for hiding this comment

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

Remove this patch and it builds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does version 2.0.19 build (see #349530)?

Copy link
Member

Choose a reason for hiding this comment

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

Why is that? This patch is dedicated to fix build issue on macOS, which is already fixed by upstream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you revert to the hash sha256-Vs0blV2IhnlEAm0WtOartz+0vLesJfp78FNJCivRxHk= you are building 2.0.18. That is why I am asking if 2.0.19 builds. Then we will know what upstream version introduced the problem.

Copy link
Member

Choose a reason for hiding this comment

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

@Aleksanaa means removing the Darwin‐only patch in patches, not removing the part of this diff that updates the version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, I am a little confused with how Github shows me 9 lines of code before the 1 line that @Aleksanaa was referring to. The colored diff is what stands out, not the last line.

OK, then I will update this PR to remove this patch.

@datafoo datafoo force-pushed the mosquitto branch 2 times, most recently from 7b773fb to b76e567 Compare October 18, 2024 13:31
datafoo
datafoo commented Oct 18, 2024
View reviewed changes
pkgs/servers/mqtt/mosquitto/default.nix
Comment on lines -42 to -50
patches = lib.optionals stdenv.hostPlatform.isDarwin [
(fetchpatch {
name = "revert-cmake-shared-to-module.patch"; # See https://github.com/eclipse/mosquitto/issues/2277
url = "https://github.com/eclipse/mosquitto/commit/e21eaeca37196439b3e89bb8fd2eb1903ef94845.patch";
sha256 = "14syi2c1rks8sl2aw09my276w45yq1iasvzkqcrqwy4drdqrf069";
revert = true;
})
];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@LeSuisse you added the patch section in commit ded0f0e, do you have any inputs on removing it?

Aleksanaa
Aleksanaa approved these changes Oct 23, 2024
View reviewed changes
Copy link
Member

@Aleksanaa Aleksanaa left a comment

Choose a reason for hiding this comment

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

4 packages built:
chickenPackages_5.chickenEggs.mosquitto mosquitto mosquitto.dev mosquitto.lib

@Aleksanaa Aleksanaa merged commit ca91897 into NixOS:master Oct 23, 2024
27 of 28 checks passed
@github-actions GitHub Actions
Copy link
Contributor

Backport failed for release-24.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-24.05
git worktree add -d .worktree/backport-349233-to-release-24.05 origin/release-24.05
cd .worktree/backport-349233-to-release-24.05
git switch --create backport-349233-to-release-24.05
git cherry-pick -x bff96234a226ba495083176ebc3edded0698728b

@datafoo datafoo mentioned this pull request Oct 23, 2024
Open
13 tasks
@datafoo datafoo deleted the mosquitto branch October 23, 2024 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilazy emilazy emilazy left review comments

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@Aleksanaa Aleksanaa Aleksanaa approved these changes

@kirillrdy kirillrdy kirillrdy approved these changes

@peterhoeg peterhoeg Awaiting requested review from peterhoeg

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 backport release-24.05 Backport PR automatically
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@datafoo @kirillrdy @Aleksanaa @SuperSandro2000 @emilazy