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.

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

matomo: Fix consistency check #73577

Merged
merged 1 commit into from Nov 27, 2019
Merged

matomo: Fix consistency check #73577

merged 1 commit into from Nov 27, 2019

Conversation

dasJ
Copy link
Member

@dasJ dasJ commented Nov 17, 2019

This is done by recalculating sizes and md5 hashes and inserting them
into the manifest.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@dasJ dasJ changed the title matomo: Fix consistentcy check matomo: Fix consistency check Nov 17, 2019
@dasJ
Copy link
Member Author

dasJ commented Nov 17, 2019

@GrahamcOfBorg build matomo matomo-beta

@ofborg ofborg bot requested review from Kiwi and florianjacob November 17, 2019 19:05
Kiwi
Kiwi suggested changes Nov 18, 2019
View reviewed changes
Copy link
Member

@Kiwi Kiwi left a comment

Choose a reason for hiding this comment

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

I've confirmed on my server that the Integrity check now passes. Thank you!

I did notice something that was not your doing while reviewing that might as well be fixed right now... There's a typo here s/evertything/everything/

@dasJ
matomo: Fix consistency check
08b9cac 
This is done by recalculating sizes and md5 hashes and inserting them
into the manifest.
@dasJ
Copy link
Member Author

dasJ commented Nov 18, 2019

@Kiwi done

@ofborg ofborg bot requested a review from Kiwi November 18, 2019 14:01
@florianjacob
Copy link
Contributor

@GrahamcOfBorg test matomo matomo-beta

@florianjacob
Copy link
Contributor

@dasJ good stuff, thanks a lot for your contribution! 👍 I never had the time to sit down and fix this annoying consistency issues.

While it's obvious that plugins/Installation/FormDatabaseSetup.php's integrity check fails due to the patch we apply in the package, I am not sure why the other files differ. Did you find out what's happening there during your research? It would be nice to have the reason for the required filesToFix as a comment. Apart from that, this is ready to merge. :)

@dasJ
Copy link
Member Author

dasJ commented Nov 18, 2019

Well, it's due to the fixupPhase:

patching script interpreter paths in /nix/store/3rswdg5487qvg8v1g757bf01lprqkx4y-matomo-3.11.0
/nix/store/3rswdg5487qvg8v1g757bf01lprqkx4y-matomo-3.11.0/share/misc/cron/archive.sh: interpreter directive changed from "/bin/sh -e" to "/nix/store/506nnycf7nk22x7n07mjjjl2g8nifpda-bash-4.4-p23/bin/sh -e"
/nix/store/3rswdg5487qvg8v1g757bf01lprqkx4y-matomo-3.11.0/share/misc/composer/clean-xhprof.sh: interpreter directive changed from "/bin/bash" to "/nix/store/506nnycf7nk22x7n07mjjjl2g8nifpda-bash-4.4-p23/bin/bash"
/nix/store/3rswdg5487qvg8v1g757bf01lprqkx4y-matomo-3.11.0/share/misc/composer/build-xhprof.sh: interpreter directive changed from "/bin/bash" to "/nix/store/506nnycf7nk22x7n07mjjjl2g8nifpda-bash-4.4-p23/bin/bash"
/nix/store/3rswdg5487qvg8v1g757bf01lprqkx4y-matomo-3.11.0/share/vendor/szymach/c-pchart/coverage.sh: interpreter directive changed from "/bin/bash" to "/nix/store/506nnycf7nk22x7n07mjjjl2g8nifpda-bash-4.4-p23/bin/bash"
/nix/store/3rswdg5487qvg8v1g757bf01lprqkx4y-matomo-3.11.0/share/vendor/pear/archive_tar/sync-php4: interpreter directive changed from "/bin/sh" to "/nix/store/506nnycf7nk22x7n07mjjjl2g8nifpda-bash-4.4-p23/bin/sh"
/nix/store/3rswdg5487qvg8v1g757bf01lprqkx4y-matomo-3.11.0/share/vendor/leafo/lessphp/package.sh: interpreter directive changed from "/bin/sh" to "/nix/store/506nnycf7nk22x7n07mjjjl2g8nifpda-bash-4.4-p23/bin/sh"

@dasJ
Copy link
Member Author

dasJ commented Nov 18, 2019

Also, I didn't really bother to find out where the inconsitencies came from, I just took the files Matomo complained about and added them to filesToFix

Kiwi
Kiwi approved these changes Nov 18, 2019
View reviewed changes
Copy link
Member

@Kiwi Kiwi left a comment

Choose a reason for hiding this comment

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

Well I'm happy now. :) Thanks

@florianjacob
Copy link
Contributor

Oh right, the remaining files are all shell or php scripts with a shebang that is patched from /bin/sh or whatever to the correct nixos paths, that explains all the other files.

@mmilata mmilata mentioned this pull request Nov 18, 2019
Merged
10 tasks
@florianjacob
Copy link
Contributor

@aanderse do you have the time to take a look? :)

@Kiwi Kiwi mentioned this pull request Nov 27, 2019
Merged
10 tasks
@aanderse
Copy link
Member

@florianjacob I don't have any ability to provide review for this PR. That being said... @dasJ wrote it, you and @Kiwi approved it... That is a merge in my books 👍

@aanderse aanderse merged commit 5988940 into NixOS:master Nov 27, 2019
@dasJ dasJ deleted the matomo-consistency branch December 8, 2019 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kiwi Kiwi Kiwi approved these changes

@florianjacob florianjacob florianjacob approved these changes

Assignees
No one assigned
Labels
6.topic: nixos 8.has: documentation 8.has: module (update) 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.

None yet
4 participants
@dasJ @florianjacob @aanderse @Kiwi