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

odoo: fix tests and update to 18.0 #346397

Merged
merged 8 commits into from
Oct 25, 2024
Merged

odoo: fix tests and update to 18.0 #346397

merged 8 commits into from
Oct 25, 2024

Conversation

phanirithvij
Copy link
Member

@phanirithvij phanirithvij commented Oct 4, 2024 •
edited
Loading

redo #344173

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.

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Oct 4, 2024
@ofborg ofborg bot requested review from mkg20001 and siriobalmelli October 4, 2024 11:31
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 8, 2024
siriobalmelli
siriobalmelli suggested changes Oct 9, 2024
View reviewed changes
Copy link
Contributor

@siriobalmelli siriobalmelli left a comment

Choose a reason for hiding this comment

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

I get a merge conflict with master:

Auto-merging pkgs/by-name/od/odoo15/package.nix
CONFLICT (content): Merge conflict in pkgs/by-name/od/odoo15/package.nix
Auto-merging pkgs/development/python-modules/aiohttp/default.nix
Automatic merge failed; fix conflicts and then commit the result.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 9, 2024
@phanirithvij
Copy link
Member Author

fixed the conflict, try again

@Erethon
Copy link
Contributor

Erethon commented Oct 9, 2024

I tried this yesterday (before the newest changes) by cherry-picking the commits and applying them on master. It pretty much worked, Odoo 17 was functional, although some smaller features of it weren't working (like QR code generation), but I believe these weren't working in the past either. From my point of view, this fixes the issues of the existing package, so thank you!

@ofborg ofborg bot requested a review from siriobalmelli October 9, 2024 10:45
@siriobalmelli
Copy link
Contributor

Screenshot 2024-10-09 at 4 40 38 PM

... still running nixpkgs-review ... 🔢

@siriobalmelli
Copy link
Contributor

Result of nixpkgs-review pr 346397 run on aarch64-darwin 1

2 packages failed to build:
  • stig
  • stig.dist

@SuperSandro2000
Copy link
Member

I don't think the PR needs to target staging.

SuperSandro2000
SuperSandro2000 reviewed Oct 9, 2024
View reviewed changes
pkgs/development/python-modules/aiohttp/default.nix Outdated Show resolved Hide resolved
pkgs/by-name/od/odoo16/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/od/odoo16/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/od/odoo15/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/od/odoo15/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/od/odoo15/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/od/odoo/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/od/odoo/package.nix Outdated Show resolved Hide resolved
@siriobalmelli
Copy link
Contributor

I don't think the PR needs to target staging.

That might explain how long it's taking me to build everything:

Screenshot 2024-10-10 at 12 12 08 AM

@SuperSandro2000
Copy link
Member

Your rebuild is taking so long because you are targeting staging which has almost no cache. There is no reason to target staging because the rebuild count is almost nothing.

@siriobalmelli
Copy link
Contributor

Result of nixpkgs-review pr 346397 run on aarch64-linux 1

10 packages failed to build:
  • odoo
  • odoo.dist
  • odoo15
  • odoo15.dist
  • odoo16
  • odoo16.dist
  • pytrainer
  • pytrainer.dist
  • stig
  • stig.dist

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: kernel The Linux kernel 8.has: module (update) This PR changes an existing module in `nixos/` labels Oct 10, 2024
@phanirithvij phanirithvij changed the base branch from staging to master October 10, 2024 15:27
@github-actions github-actions bot removed 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: kernel The Linux kernel 8.has: module (update) This PR changes an existing module in `nixos/` labels Oct 10, 2024
@phanirithvij
Copy link
Member Author

phanirithvij commented Oct 10, 2024 •
edited
Loading

Seems like odoo has dropped 15.0 for their docker images and released 18 as well, see https://github.com/odoo/docker and https://github.com/docker-library/docs/tree/master/odoo

  • I updated the target branch to master
  • updated odoo to 18
  • added odoo17/package.nix
  • modified the update script to handle updating all versions not just the latest version
  • made changes suggested by sandro

Should we remove odoo 15 or still keep it?

after git cherry-pick 222d7063940ddf7d60b86ad81d54663b4357254a

❯ echo odoo{15,16,17,} | xargs -n1 nix-build --no-out-link -A
/nix/store/1d5fl9ki4jzjcnl0z8jvkvar9r6hd6kx-odoo-15.0.20241010
/nix/store/cnfhfq4axq2jchifx5lhalhnlpicap3y-odoo-16.0.20241010
/nix/store/0l9vsxsvkyw4w5mapl5rfxwkb7bxbjb1-odoo-17.0.20241010
/nix/store/8i1ybfms4n9i5af9fpnhx6x4l8szkaxh-odoo-18.0.20241010
❯ echo nixosTests.odoo{15,16,17,} | xargs -n1 nix-build --no-out-link -A
/nix/store/clc8pdnifl3hbqqma2wkw7rbm0lawfli-vm-test-run-odoo
/nix/store/1n8j55q0c8rmy17wq5vc0dgn8g09xqb8-vm-test-run-odoo
/nix/store/864fffbpjzfd3r02r2d7ns53l88phx1p-vm-test-run-odoo
/nix/store/dfnqjljyzy56jsp87w6hp0s9qb6hvvfy-vm-test-run-odoo

EDIT: never mind, still WIP since symlinks not allowed for upper directory for pkgs/by-name, I'll see what I can do to avoid code duplication.

@siriobalmelli
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 346397

aarch64-linux

❌ 10 packages failed to build:
  • odoo15
  • odoo15.dist
  • odoo16
  • odoo16.dist
  • odoo17
  • odoo17.dist
  • pytrainer
  • pytrainer.dist
  • stig
  • stig.dist
✅ 2 packages built:
  • odoo
  • odoo.dist

@phanirithvij
Copy link
Member Author

phanirithvij commented Oct 13, 2024 •
edited
Loading

I tried nix-build -A pkgsCross.aarch64-multiplatform.odoo on my x86_64-linux but can't do it with 16GB ram + 16GB swap.

Edit: works with ~30GB swap. nix-build command takes that much ram?

@phanirithvij phanirithvij mentioned this pull request Oct 14, 2024
Closed
13 tasks
@phanirithvij
Copy link
Member Author

phanirithvij commented Oct 16, 2024 •
edited
Loading

This is ready, but the update scripts are duplicated, since the pkgs/by-name doesn't allow accessing files outside the tree. #292214
I doubt we want to move these back to all-packages. So I think we have to live with this duplication (all odoo update scripts are the exact same file).

Can everyone test it, try to use odoo18 (if possible older versions too) for a few days and report any breakages here. I hope to get this in before 24.11 freeze. (~ 1 week)
No need to cherry-pick the furl commit, it is now in master.

NOTE: odoo15 will be dropped after 24.11 release.

@phanirithvij phanirithvij changed the title odoo: fix tests and update take 2 odoo: fix tests and update to 18.0 Oct 16, 2024
phanirithvij and others added 8 commits October 23, 2024 10:22
@phanirithvij
python310Packages.aiohttp: disable failing test
ce2c071 
Signed-off-by: phanirithvij <phanirithvij2000@gmail.com>
@mweinelt @phanirithvij
python310Packages.uvloop: disable failing test
30218c5
@mweinelt @phanirithvij
python310Packages.websockets: disable failing test
a1a6f4a
@phanirithvij @dotlambda
odoo: update to 18.0, enable tests
90f8329 
Co-authored-by: Robert Schütz <github@dotlambda.de>
Signed-off-by: phanirithvij <phanirithvij2000@gmail.com>
@phanirithvij
odoo17: init
4b7c3e6 
Signed-off-by: phanirithvij <phanirithvij2000@gmail.com>
@phanirithvij
odoo16: update and enable tests
7665a47 
Signed-off-by: phanirithvij <phanirithvij2000@gmail.com>
@phanirithvij
odoo15: update and enable tests
34e7279 
Signed-off-by: phanirithvij <phanirithvij2000@gmail.com>
@phanirithvij
odoo: update script for all versions
d48355b 
Signed-off-by: phanirithvij <phanirithvij2000@gmail.com>
@phanirithvij phanirithvij marked this pull request as ready for review October 23, 2024 06:46
@siriobalmelli
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 346397

x86_64-linux

✅ 14 packages built:
  • circus
  • circus.dist
  • odoo
  • odoo.dist
  • odoo15
  • odoo15.dist
  • odoo16
  • odoo16.dist
  • odoo17
  • odoo17.dist
  • pytrainer
  • pytrainer.dist
  • stig
  • stig.dist

@mkg20001
Copy link
Member

Any objections to merge? Would merge in a few days

@siriobalmelli
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 346397

aarch64-linux

❌ 12 packages failed to build:
  • circus
  • circus.dist
  • odoo15
  • odoo15.dist
  • odoo16
  • odoo16.dist
  • odoo17
  • odoo17.dist
  • pytrainer
  • pytrainer.dist
  • stig
  • stig.dist
✅ 2 packages built:
  • odoo
  • odoo.dist

@siriobalmelli
Copy link
Contributor

LGTM; I'll circle back to the aarch64-linux builds failures later as needed.

@phanirithvij
Copy link
Member Author

Cross compilation works tho

$ echo pkgsCross.aarch64-multiplatform.odoo{15,16,17,} | xargs -n1 nix build -f . --print-out-paths
/nix/store/7m2j0svg34vjnhqm8gyb21l3fz9fyyjm-odoo-15.0.20241010-aarch64-unknown-linux-gnu
/nix/store/68jbpcb3620kg5cw338zwbvax5yjzqnh-odoo-16.0.20241010-aarch64-unknown-linux-gnu
/nix/store/x0wq3s1dfbmwhgn519jw3mhxgizcmyh7-odoo-17.0.20241010-aarch64-unknown-linux-gnu
/nix/store/gagv0a46hbw8lg639y9fx0djjl67cksh-odoo-18.0.20241010-aarch64-unknown-linux-gnu

@mkg20001 mkg20001 merged commit e907f6a into NixOS:master Oct 25, 2024
30 of 31 checks passed
@phanirithvij phanirithvij deleted the odoo-fix-tests branch October 26, 2024 04:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@mweinelt mweinelt mweinelt left review comments

@siriobalmelli siriobalmelli siriobalmelli approved these changes

@mkg20001 mkg20001 Awaiting requested review from mkg20001

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@phanirithvij @Erethon @siriobalmelli @SuperSandro2000 @mkg20001 @mweinelt