Skip to content

Conversation

@gonsolo
Copy link
Contributor

@gonsolo gonsolo commented Jan 7, 2026

  • Fix 'std::iterator' deprecation warnings by defining iterator traits explicitly.
  • Fix 'std::allocator::construct' removal in C++20 using std::allocator_traits.
  • Resolve uninitialized variable warnings in dim2::Point.
  • Add [[fallthrough]] attributes to intentional switch case transitions.

@gonsolo gonsolo requested review from bouk and flokli and removed request for flokli January 7, 2026 14:43
@nixpkgs-ci nixpkgs-ci bot requested a review from trepetti January 7, 2026 14:48
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. labels Jan 7, 2026
Copy link
Member

@bouk bouk left a comment

Choose a reason for hiding this comment

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

not sure why you added me as a reviewer since I'm not a maintainer, but it doesn't seem a good idea to me to switch to a fork

@gonsolo gonsolo marked this pull request as draft January 7, 2026 15:37
@gonsolo
Copy link
Contributor Author

gonsolo commented Jan 7, 2026

not sure why you added me as a reviewer since I'm not a maintainer, but it doesn't seem a good idea to me to switch to a fork

There were two reviewers suggested in the corner top right. Sorry about that. I converted this PR to a draft since it is WIP.

@gonsolo gonsolo marked this pull request as ready for review January 7, 2026 16:06
@gonsolo
Copy link
Contributor Author

gonsolo commented Jan 7, 2026

but it doesn't seem a good idea to me to switch to a fork

Version 1.3.1 was released in 2014. The fork from OpenROAD is at least updated to compile.

@gonsolo
Copy link
Contributor Author

gonsolo commented Jan 7, 2026

Could you review, @eljamm?

@eljamm
Copy link
Contributor

eljamm commented Jan 7, 2026

lemon-graph doesn't appear to be broken:

$ hydra-check lemon-graph
Build Status for nixpkgs.lemon-graph.x86_64-linux on jobset nixos/trunk-combined
https://hydra.nixos.org/job/nixos/trunk-combined/nixpkgs.lemon-graph.x86_64-linux
✔           lemon-graph-1.3.1  2025-12-31  https://hydra.nixos.org/build/317865215
✔           lemon-graph-1.3.1  2025-11-23  https://hydra.nixos.org/build/314192137
✔           lemon-graph-1.3.1  2025-11-13  https://hydra.nixos.org/build/313176240
✔           lemon-graph-1.3.1  2025-10-21  https://hydra.nixos.org/build/310404901
✔           lemon-graph-1.3.1  2025-10-07  https://hydra.nixos.org/build/309254064
✖ (Failed)  lemon-graph-1.3.1  2025-10-05  https://hydra.nixos.org/build/308844417
✔           lemon-graph-1.3.1  2025-09-11  https://hydra.nixos.org/build/306922703
✔           lemon-graph-1.3.1  2025-08-25  https://hydra.nixos.org/build/305602029
✔           lemon-graph-1.3.1  2025-07-27  https://hydra.nixos.org/build/303380753
✔           lemon-graph-1.3.1  2025-07-14  https://hydra.nixos.org/build/302408273

And I can indeed build it from the latest master:

nix build github:nixos/nixpkgs/master#lemon-graph -L
* Fix 'std::iterator' deprecation warnings by defining iterator traits explicitly.

* Fix 'std::allocator::construct' removal in C++20 using std::allocator_traits.

* Resolve uninitialized variable warnings in dim2::Point.

* Add [[fallthrough]] attributes to intentional switch case transitions.

If these are just warnings, I think we can probably ignore them for now since they don't appear to be breaking the build.

@gonsolo
Copy link
Contributor Author

gonsolo commented Jan 8, 2026

lemon-graph doesn't appear to be broken:

Ah, yes, it's more complicated than that; while trying to get Librelane into Nix, I had to update OpenROAD:
https://github.com/NixOS/nixpkgs/pull/477789/changes
This fails among other things because of lemon-graph.

If these are just warnings, I think we can probably ignore them for now since they don't appear to be breaking the build.

I fixed everything upstream in the OpenROAD fork:
The-OpenROAD-Project/lemon-graph#4

I also contacted the author of Lemon at lemon.cs.elte.hu, you may have received the email.
Hopefully he can incorporate the changes, or if development is abandoned we can switch to the OpenROAD fork.

Thanks anyway,
g

@gonsolo
Copy link
Contributor Author

gonsolo commented Jan 8, 2026

The error from an updated openroad is:

error: Cannot build '/nix/store/bg07lclj48imcsc95b2k8dq9nrqsyxqj-openroad-2.0-unstable-2026-01-07.drv'.
       Reason: builder failed with exit code 2.
       Output paths:
         /nix/store/p31xf1kl9rl0j8pyx1ahvmx07jmbp850-openroad-2.0-unstable-2026-01-07
       Last 25 log lines:
       >   289 |     virtual void erase(const std::vector<Key>& keys) {
       >       |                  ^~~~~
       > /nix/store/kdlh4zmf3ycdnx4pmi0fmgkn0mry04l6-lemon-graph-1.3.1/include/lemon/bits/array_map.h:292:19: error: 'lemon::ArrayMap<lemon::GraphExtender<lemon::ListGraphBase>, lemon::ListGraphBase::Node, odb::Point>::Allocator' {aka 'class std::allocator<odb::Point>'} has no member named 'destroy'
       >   292 |         allocator.destroy(&(values[id]));
       >       |         ~~~~~~~~~~^~~~~~~

This is not triggered in lemon-graph since it is header only and not in test.

@gonsolo
Copy link
Contributor Author

gonsolo commented Jan 8, 2026

I took a different path now; just vendor the necessary patch from the OpenROAD fork.
I hope this is acceptible.
Since this is one of the last 7 PRs to merge Librelane, could you review, @eljamm @donn?

@gonsolo gonsolo mentioned this pull request Jan 8, 2026
13 tasks
@eljamm
Copy link
Contributor

eljamm commented Jan 8, 2026

nixpkgs-review result for #477756

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 477756
Commit: 7ba184ad35bea0ba7de54ae409a3238fc16bbe61 (subsequent changes)
Merge: e9d2d6ac06c55ddf79ec535580e0b8e268d504ee

Logs: https://github.com/eljamm/nixpkgs-review-gha/actions/runs/20813070935


x86_64-linux

❌ 2 packages failed to build:
  • coloquinte
  • openroad
✅ 2 packages built:
  • lemon-graph
  • openmvg

Error logs: `x86_64-linux`
coloquinte
@nix { "action": "setPhase", "phase": "buildPhase" }
build flags: -j4 SHELL=/nix/store/lw117lsr8d585xs63kx5k233impyrq7q-bash-5.3p3/bin/bash
[  2%] Building CXX object CMakeFiles/coloquinte.dir/src/coloquinte.cpp.o
[  8%] Building CXX object CMakeFiles/coloquinte.dir/src/place_global/net_model.cpp.o
[  8%] Building CXX object CMakeFiles/coloquinte.dir/src/parameters.cpp.o
[ 11%] Building CXX object CMakeFiles/coloquinte.dir/src/place_global/density_legalizer.cpp.o
[ 14%] Building CXX object CMakeFiles/coloquinte.dir/src/place_global/density_grid.cpp.o
[ 17%] Building CXX object CMakeFiles/coloquinte.dir/src/place_global/place_global.cpp.o
[ 20%] Building CXX object CMakeFiles/coloquinte.dir/src/place_detailed/legalizer.cpp.o
[ 22%] Building CXX object CMakeFiles/coloquinte.dir/src/place_detailed/abacus_legalizer.cpp.o
[ 25%] Building CXX object CMakeFiles/coloquinte.dir/src/place_detailed/tetris_legalizer.cpp.o
[ 28%] Building CXX object CMakeFiles/coloquinte.dir/src/place_detailed/row_legalizer.cpp.o
/build/source/src/place_detailed/tetris_legalizer.cpp: In member function 'std::pair<bool, int> coloquinte::TetrisLegalizer::attemptPlacement(int, int) const':
/build/source/src/place_detailed/tetris_legalizer.cpp:48:20: error: 'clamp' is not a member of 'std'
   48 |     int pos = std::clamp(x, b, e);
      |                    ^~~~~
make[2]: *** [CMakeFiles/coloquinte.dir/build.make:191: CMakeFiles/coloquinte.dir/src/place_detailed/tetris_legalizer.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/Makefile2:133: CMakeFiles/coloquinte.dir/all] Error 2
make: *** [Makefile:146: all] Error 2
openroad
 1325 |              block->dbuAreaToMicrons(stdInstsArea_));
      |                                      ^~~~~~~~~~~~~
/build/source/src/gpl/src/placerBase.cpp:1331:38: error: 'macroInstsArea_' was not declared in this scope
 1331 |              block->dbuAreaToMicrons(macroInstsArea_));
      |                                      ^~~~~~~~~~~~~~~
/build/source/src/gpl/src/placerBase.cpp: At global scope:
/build/source/src/gpl/src/placerBase.cpp:1345:9: error: no declaration matches 'int64_t gpl::PlacerBase::macroInstsArea() const'
 1345 | int64_t PlacerBase::macroInstsArea() const
      |         ^~~~~~~~~~
/build/source/src/gpl/src/placerBase.cpp:1345:9: note: no functions named 'int64_t gpl::PlacerBase::macroInstsArea() const'
/build/source/src/gpl/src/placerBase.h:378:7: note: 'class gpl::PlacerBase' defined here
  378 | class PlacerBase
      |       ^~~~~~~~~~
make[2]: *** [src/gpl/CMakeFiles/gpl.dir/build.make:161: src/gpl/CMakeFiles/gpl.dir/src/placerBase.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/Makefile2:5275: src/gpl/CMakeFiles/gpl.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
[ 91%] Built target rmp
[ 94%] Built target drt_lib
make: *** [Makefile:146: all] Error 2

aarch64-linux

❌ 2 packages failed to build:
  • coloquinte
  • openroad
✅ 2 packages built:
  • lemon-graph
  • openmvg

Error logs: `aarch64-linux`
coloquinte
@nix { "action": "setPhase", "phase": "buildPhase" }
build flags: -j4 SHELL=/nix/store/q1lr86rzsxxbbqdz8m6bcv94saaz8wlv-bash-5.3p3/bin/bash
[  2%] Building CXX object CMakeFiles/coloquinte.dir/src/parameters.cpp.o
[  5%] Building CXX object CMakeFiles/coloquinte.dir/src/coloquinte.cpp.o
[  8%] Building CXX object CMakeFiles/coloquinte.dir/src/place_global/density_legalizer.cpp.o
[ 11%] Building CXX object CMakeFiles/coloquinte.dir/src/place_global/net_model.cpp.o
[ 14%] Building CXX object CMakeFiles/coloquinte.dir/src/place_global/density_grid.cpp.o
[ 17%] Building CXX object CMakeFiles/coloquinte.dir/src/place_global/place_global.cpp.o
[ 20%] Building CXX object CMakeFiles/coloquinte.dir/src/place_detailed/legalizer.cpp.o
[ 22%] Building CXX object CMakeFiles/coloquinte.dir/src/place_detailed/abacus_legalizer.cpp.o
[ 25%] Building CXX object CMakeFiles/coloquinte.dir/src/place_detailed/tetris_legalizer.cpp.o
[ 28%] Building CXX object CMakeFiles/coloquinte.dir/src/place_detailed/row_legalizer.cpp.o
/build/source/src/place_detailed/tetris_legalizer.cpp: In member function 'std::pair<bool, int> coloquinte::TetrisLegalizer::attemptPlacement(int, int) const':
/build/source/src/place_detailed/tetris_legalizer.cpp:48:20: error: 'clamp' is not a member of 'std'
   48 |     int pos = std::clamp(x, b, e);
      |                    ^~~~~
make[2]: *** [CMakeFiles/coloquinte.dir/build.make:191: CMakeFiles/coloquinte.dir/src/place_detailed/tetris_legalizer.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/Makefile2:133: CMakeFiles/coloquinte.dir/all] Error 2
make: *** [Makefile:146: all] Error 2
openroad
      |                              nonPlaceInsts_
/build/source/src/gpl/src/placerBase.cpp:1325:38: error: 'stdInstsArea_' was not declared in this scope
 1325 |              block->dbuAreaToMicrons(stdInstsArea_));
      |                                      ^~~~~~~~~~~~~
/build/source/src/gpl/src/placerBase.cpp:1331:38: error: 'macroInstsArea_' was not declared in this scope
 1331 |              block->dbuAreaToMicrons(macroInstsArea_));
      |                                      ^~~~~~~~~~~~~~~
/build/source/src/gpl/src/placerBase.cpp: At global scope:
/build/source/src/gpl/src/placerBase.cpp:1345:9: error: no declaration matches 'int64_t gpl::PlacerBase::macroInstsArea() const'
 1345 | int64_t PlacerBase::macroInstsArea() const
      |         ^~~~~~~~~~
/build/source/src/gpl/src/placerBase.cpp:1345:9: note: no functions named 'int64_t gpl::PlacerBase::macroInstsArea() const'
/build/source/src/gpl/src/placerBase.h:378:7: note: 'class gpl::PlacerBase' defined here
  378 | class PlacerBase
      |       ^~~~~~~~~~
make[2]: *** [src/gpl/CMakeFiles/gpl.dir/build.make:161: src/gpl/CMakeFiles/gpl.dir/src/placerBase.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:5275: src/gpl/CMakeFiles/gpl.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
[ 94%] Built target drt_lib
make: *** [Makefile:146: all] Error 2

x86_64-darwin (sandbox = relaxed)

✅ 1 package built:
  • lemon-graph

aarch64-darwin (sandbox = relaxed)

❌ 1 package failed to build:
  • openroad
✅ 2 packages built:
  • lemon-graph
  • openmvg

Copy link
Contributor

@eljamm eljamm left a comment

Choose a reason for hiding this comment

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

I took a different path now; just vendor the necessary patch from the OpenROAD fork.
I hope this is acceptible.

Yes, I think that's a much better solution.

x86_64-linux

❌ 2 packages failed to build:

* coloquinte

* openroad

Openroad is already broken with an open PR to fix it and Coloquinte will potentially work after it's been updated:

This PR looks good to me now, but let's wait until these 2 are fixed before we merge.

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jan 8, 2026
@gonsolo gonsolo mentioned this pull request Jan 8, 2026
2 tasks
@gonsolo
Copy link
Contributor Author

gonsolo commented Jan 9, 2026

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 477756
Commit: 9427e76a5a4b044e7dde61dfe79962b3b4b06afe


x86_64-linux

✅ 4 packages built:
  • coloquinte
  • lemon-graph
  • openmvg
  • openroad

@gonsolo
Copy link
Contributor Author

gonsolo commented Jan 9, 2026

I think this is ready, maybe @eljamm @imincik @GaetanLepage?

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 3+ This PR was reviewed and approved by three or more persons. 2.status: merge-bot eligible This PR can be merged by commenting "@NixOS/nixpkgs-merge-bot merge". and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Jan 9, 2026
@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 477756 --package lemon-graph
Commit: 9427e76a5a4b044e7dde61dfe79962b3b4b06afe


x86_64-linux

✅ 1 package built:
  • lemon-graph

aarch64-linux

✅ 1 package built:
  • lemon-graph

x86_64-darwin

✅ 1 package built:
  • lemon-graph

aarch64-darwin

✅ 1 package built:
  • lemon-graph

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 477756
Commit: 9427e76a5a4b044e7dde61dfe79962b3b4b06afe


x86_64-linux

✅ 4 packages built:
  • coloquinte
  • lemon-graph
  • openmvg
  • openroad

@GaetanLepage GaetanLepage added this pull request to the merge queue Jan 9, 2026
Merged via the queue into NixOS:master with commit ff0e0b1 Jan 9, 2026
30 of 34 checks passed
@gonsolo gonsolo deleted the lemon-graph branch January 10, 2026 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge-bot eligible This PR can be merged by commenting "@NixOS/nixpkgs-merge-bot merge". 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 3+ This PR was reviewed and approved by three or more persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants