Skip to content

arangodb: 3.4.8 -> 3.10.0#194670

Merged
vcunat merged 24 commits intoNixOS:masterfrom
jsoo1:jsoo1/arangodb-bump
Nov 7, 2022
Merged

arangodb: 3.4.8 -> 3.10.0#194670
vcunat merged 24 commits intoNixOS:masterfrom
jsoo1:jsoo1/arangodb-bump

Conversation

@jsoo1
Copy link
Contributor

@jsoo1 jsoo1 commented Oct 5, 2022

Description of changes

arangodb was out of date (last commited 2018). This brings it to a more current version.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 2.status: merge conflict This PR has merge conflicts with the target branch 8.has: clean-up This PR removes packages or removes other cruft and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 5, 2022
@ofborg ofborg bot requested a review from flosse October 6, 2022 00:08
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Oct 6, 2022
@jsoo1 jsoo1 changed the title arangodb: 3.4.8 -> 3.9.3 arangodb: 3.4.8 -> 3.10.0 Oct 6, 2022
@jsoo1
Copy link
Contributor Author

jsoo1 commented Oct 6, 2022

Turns out 3.9.3 -> 3.10.0 wasn't too bad.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation labels Oct 6, 2022
@ofborg ofborg bot removed the 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. label Oct 6, 2022
@yorickvP yorickvP added the 1.severity: security Issues which raise a security issue, or PRs that fix one label Oct 10, 2022
@jsoo1
Copy link
Contributor Author

jsoo1 commented Oct 10, 2022

@risicle
Copy link
Contributor

risicle commented Oct 10, 2022

Very unlikely, I think @yorickvP 's point is this will fix those issues.

@yorickvP
Copy link
Contributor

Yep, this upgrade will fix those issues.

@jsoo1
Copy link
Contributor Author

jsoo1 commented Oct 12, 2022

Yep, this upgrade will fix those issues.

I see, thanks for the clarification!

@vcunat
Copy link
Member

vcunat commented Oct 25, 2022

That reasoning doesn't make sense to me. You'd get gcc11 (which is supported by upstream). Instead you force gcc10, with an argument speaking against such a change.

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 25, 2022
@vcunat
Copy link
Member

vcunat commented Oct 25, 2022

It (now) doesn't build for me on x86_64 NixOS. Maybe there was some (other) reason to downgrade compiler version. The errors look like changes around standard strictness, etc. (complex C++ traces; probably no sense to post them here)

@jsoo1
Copy link
Contributor Author

jsoo1 commented Oct 25, 2022

It (now) doesn't build for me on x86_64 NixOS. Maybe there was some (other) reason to downgrade compiler version. The errors look like changes around standard strictness, etc. (complex C++ traces; probably no sense to post them here)

Same. I will investigate.

@jsoo1
Copy link
Contributor Author

jsoo1 commented Oct 27, 2022

I found a new problem - maybe or maybe not related to compile failures - /proc/cpuinfo is read unconditionally for architecture feature detection. You turn this off by specifying -DTARGET_ARCHITECTURE=xyz up front. However, -DTARGET_ARCHITECTURE=generic seems like the best choice and that causes the vendored v8 not to compile.

edit: see arangodb/arangodb@cc860c3

jsoo1 and others added 19 commits October 30, 2022 10:20
It fails to apply
Co-authored-by: Robert Scott <code@humanleg.org.uk>
USE_OPTIMIZE_FOR_ARCHITECTURE was removed without commit in a prior
commit. This is because it was removed upstream:

arangodb/arangodb@cc860c3

This is unfortunate - now /proc/cpuinfo is read unconditionally for
feature detection.

Some other means of feature detection will have to be found.
It seems that gcc11 - though reportedly supported - introduced some
constraint that arangodb fails to compile with.
Avoid non-deterministic builds of ArangoDB proper. Note that this
provides 0 guarantees about the vendored libraries doing anything
similar.
If -DTARGET_ARCHITECTURE is supplied arango's cmake configuration will
not use /proc/cpuinfo for feature detection. However, `generic` and
`none` options both fail when building the vendored v8.

This workaround provides some defaults, but warns if no override is
provided.
As far as I can tell, this can cause compile failures when building
the vendored abseil_cpp if choosing a target arch that does not
support sse. This might be possible to determine programmatically, but
it is more flexible to let the user decide.
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 30, 2022
jsoo1 added 2 commits October 31, 2022 09:44
For which no reliable default target architecture is provided upstream.
@jsoo1
Copy link
Contributor Author

jsoo1 commented Oct 31, 2022

I decided to keep only x86_64-linux as the supported system for arangodb since I can't find a reasonable default value for -DTARGET_ARCHITECTURE on aarch64-linux. I documented the change and how to alter the new parameters.

Also, I used haswell as the default architecture since generic and none both fail to build and I don't really have the time to understand why. I documented this change and how to alter that behavior as well.

@jsoo1
Copy link
Contributor Author

jsoo1 commented Oct 31, 2022

This is probably about as much effort as I can give to this package at the moment. Hopefully what I have is sufficient but I would be happy to see some other eyes on this.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1378

@vcunat vcunat merged commit 7fa607b into NixOS:master Nov 7, 2022
description = "A native multi-model database with flexible data models for documents, graphs, and key-values";
license = licenses.asl20;
platforms = [ "x86_64-linux" ];
maintainers = [ maintainers.flosse maintainers.jsoo1 ];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
maintainers = [ maintainers.flosse maintainers.jsoo1 ];
maintainers = with maintainers; [ flosse jsoo1 ];

# prevent failing with "cmake-3.13.4/nix-support/setup-hook: line 10: ./3rdParty/rocksdb/RocksDBConfig.cmake.in: No such file or directory"
dontFixCmake = true;
NIX_CFLAGS_COMPILE = "-Wno-error";
preConfigure = "patchShebangs utils";
Copy link
Member

Choose a reason for hiding this comment

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

This should really be a multiline string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! #200210

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.severity: security Issues which raise a security issue, or PRs that fix one 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: clean-up This PR removes packages or removes other cruft 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants