Skip to content

convos: init at 4.22#88940

Merged
talyz merged 7 commits intoNixOS:masterfrom
stigtsp:package/convos-init
Jun 25, 2020
Merged

convos: init at 4.22#88940
talyz merged 7 commits intoNixOS:masterfrom
stigtsp:package/convos-init

Conversation

@stigtsp
Copy link
Member

@stigtsp stigtsp commented May 26, 2020 •
edited
Loading

This PR add the web based IRC client convos, including nixos module and tests.

Several perlPackage dependencies are added, and some are updated.

  • 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 nixpkgs-review --run "nixpkgs-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.

@stigtsp stigtsp requested review from etu and volth May 26, 2020 14:08
@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels May 26, 2020
stigok
stigok approved these changes May 26, 2020
View reviewed changes
@stigtsp stigtsp requested a review from aanderse June 5, 2020 11:04
@stigtsp
Copy link
Member Author

stigtsp commented Jun 5, 2020

Updating convos to 4.18

@stigtsp stigtsp force-pushed the package/convos-init branch from 4cf88ca to b4fab0f Compare June 5, 2020 12:57
@stigtsp stigtsp changed the title convos: init at 4.12 convos: init at 4.18 Jun 5, 2020
@stigtsp stigtsp mentioned this pull request Jun 5, 2020
Closed
jhthorsen
jhthorsen reviewed Jun 6, 2020
View reviewed changes
Copy link

@jhthorsen jhthorsen left a comment

Choose a reason for hiding this comment

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

This is awesome 👍 So glad you created this PR @stigtsp!

@stigtsp stigtsp force-pushed the package/convos-init branch 2 times, most recently from 201e4a7 to 51ecd4e Compare June 9, 2020 17:42
jhthorsen
jhthorsen reviewed Jun 10, 2020
View reviewed changes
@stigtsp stigtsp force-pushed the package/convos-init branch from 51ecd4e to 7d19cc0 Compare June 11, 2020 06:21
@stigtsp stigtsp changed the title convos: init at 4.18 convos: init at 4.19 Jun 11, 2020
@stigtsp stigtsp force-pushed the package/convos-init branch from 7d19cc0 to 384e35d Compare June 11, 2020 06:28
@stigtsp stigtsp requested a review from talyz June 11, 2020 06:33
@stigtsp stigtsp force-pushed the package/convos-init branch from 384e35d to b19c0b7 Compare June 11, 2020 07:17
aanderse
aanderse requested changes Jun 11, 2020
View reviewed changes
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

I have reviewed the module. Maybe @volth wants to give a quick glance over the packages for final approval there 🤷‍♂️

Looking good though 👍

@stigtsp stigtsp force-pushed the package/convos-init branch from b19c0b7 to 4115ddf Compare June 11, 2020 12:04
@stigtsp
Copy link
Member Author

stigtsp commented Jun 11, 2020

Thx for reviewing 👍

aanderse
aanderse reviewed Jun 11, 2020
View reviewed changes
nixos/modules/services/web-apps/convos.nix Outdated
Copy link
Member

@aanderse aanderse Jun 11, 2020

Choose a reason for hiding this comment

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

@stigtsp at least a few of these options are already implied with DynamicUser. A quick search for DynamicUser in the systemd manual will cut a few lines here.

Copy link
Contributor

@talyz talyz Jun 11, 2020

Choose a reason for hiding this comment

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

Yes, ProtectSystem and ProtectHome are implied and can be removed.

Copy link
Member Author

@stigtsp stigtsp Jun 11, 2020

Choose a reason for hiding this comment

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

Thanks @talyz @aanderse :) I've removed ProtectSystem=strict.

Seems like ProtectHome=read-only is implied by DynamicUser=true according to the documentation, so keeping ProtectHome=true.

Added SystemCallFilter, SystemCallArchitectures, CapabilityBoundingSet, and some more flags highlighted by systemd-analyze security.

→ Overall exposure level for convos.service: 1.3 OK 🙂

Does this look ok?

Copy link
Contributor

@talyz talyz Jun 11, 2020

Choose a reason for hiding this comment

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

Ah, yes, you're right :) Looks good to me, assuming all features you want still work as expected ;)

@stigtsp stigtsp force-pushed the package/convos-init branch from 4115ddf to 8e7faf8 Compare June 11, 2020 14:55
@talyz
Copy link
Contributor

talyz commented Jun 11, 2020

@GrahamcOfBorg test convos

@stigtsp stigtsp marked this pull request as draft June 17, 2020 15:02
@stigtsp
Copy link
Member Author

stigtsp commented Jun 17, 2020

Waiting for a new release from upstream that contains some important fixes.

@talyz
Copy link
Contributor

talyz commented Jun 17, 2020

@stigtsp Can you add the test to passthru.tests? That way the test is automatically run for PRs and easy to refer to. See tests here: https://nixos.org/nixpkgs/manual/#sec-standard-meta-attributes

@stigtsp stigtsp force-pushed the package/convos-init branch 3 times, most recently from ce47b04 to 509bac5 Compare June 21, 2020 18:43
@stigtsp stigtsp changed the title convos: init at 4.19 convos: init at 4.22 Jun 21, 2020
@stigtsp
Copy link
Member Author

stigtsp commented Jun 21, 2020

This PR includes updates to perlPackages.MojoliciousPluginOpenAPI, perlPackages.JSONValidator and perlPackages.Mojolicious which are also included in #91184 I suggest merging this PR after the cpan2nix updates have been merged.

stigtsp added 7 commits June 22, 2020 13:56
@stigtsp
perlPackages.Mojolicious: 8.32 -> 8.55
dbda655
@stigtsp
perlPackages.Mojolicious: add sgo to maintainers
1fecbbc
@stigtsp
perlPackages.JSONValidator: 3.23 -> 4.00
15f889d
@stigtsp
perlPackages.MojoliciousPluginOpenAPI: 2.21 -> 3.33
e755287
@stigtsp
convos: init at 4.22
497ac5b 
dependencies:
perlPackages.IRCUtils: init at 0.12
perlPackages.LinkEmbedder: init at 1.12
perlPackages.MojoliciousPluginWebpack: init at 0.12
perlPackages.ParseIRC: init at 1.22
perlPackages.TimePiece: init at 1.3401
perlPackages.UnicodeUTF8: init at 0.62
@stigtsp
nixos/convos: init
042a2d8
@stigtsp
nixos/convos: add test
a71fd5c
@stigtsp stigtsp force-pushed the package/convos-init branch from 509bac5 to a71fd5c Compare June 22, 2020 12:04
@stigtsp
Copy link
Member Author

stigtsp commented Jun 22, 2020 •
edited
Loading

Those mass updates are typically stuck for 2-3 weeks (no one wants to review them), so probably there is no point to wait.
Also that PR is against staging which merged to master every few weeks.

Ok - updated Mojolicious in this PR to 8.55, so it should be ready I hope :-) Does the perlPackages updates look ok?

Result of nixpkgs-review pr 88940 1

42 packages built:
- abcde
- convos
- perl528Packages.IRCUtils
- perl528Packages.JSONValidator
- perl528Packages.LinkEmbedder
- perl528Packages.MojoIOLoopForkCall
- perl528Packages.MojoJWT
- perl528Packages.MojoPg
- perl528Packages.MojoRedis
- perl528Packages.MojoSQLite
- perl528Packages.Mojolicious
- perl528Packages.MojoliciousPluginMail
- perl528Packages.MojoliciousPluginOpenAPI
- perl528Packages.MojoliciousPluginStatus
- perl528Packages.MojoliciousPluginTextExceptions
- perl528Packages.MojoliciousPluginWebpack
- perl528Packages.Mojomysql
- perl528Packages.MusicBrainz
- perl528Packages.OpenAPIClient
- perl528Packages.ParseIRC
- perl528Packages.TimePiece
- perl528Packages.UnicodeUTF8
- perl530Packages.IRCUtils
- perl530Packages.JSONValidator
- perl530Packages.LinkEmbedder
- perl530Packages.MojoIOLoopForkCall
- perl530Packages.MojoJWT
- perl530Packages.MojoPg
- perl530Packages.MojoRedis
- perl530Packages.MojoSQLite
- perl530Packages.Mojolicious
- perl530Packages.MojoliciousPluginMail
- perl530Packages.MojoliciousPluginOpenAPI
- perl530Packages.MojoliciousPluginStatus
- perl530Packages.MojoliciousPluginTextExceptions
- perl530Packages.MojoliciousPluginWebpack
- perl530Packages.Mojomysql
- perl530Packages.MusicBrainz
- perl530Packages.OpenAPIClient
- perl530Packages.ParseIRC
- perl530Packages.TimePiece
- perl530Packages.UnicodeUTF8

@stigtsp stigtsp marked this pull request as ready for review June 22, 2020 12:11
@stigtsp stigtsp requested a review from aanderse June 24, 2020 19:08
aanderse
aanderse approved these changes Jun 24, 2020
View reviewed changes
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

Yeah this looks good to me from a quick overview. @talyz I think we should merge... agree?

talyz
talyz approved these changes Jun 25, 2020
View reviewed changes
Copy link
Contributor

@talyz talyz left a comment

Choose a reason for hiding this comment

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

@aanderse Yep, looks good to me 👍

@talyz talyz merged commit c00bf08 into NixOS:master Jun 25, 2020
@talyz
Copy link
Contributor

talyz commented Jun 25, 2020

@stigtsp Thanks for doing this! Great work! 🎉

@stigtsp
Copy link
Member Author

stigtsp commented Jun 25, 2020

Thx for the reviews and advice, everyone :)

@FRidh
Copy link
Member

FRidh commented Jun 26, 2020

Fixed eval in cd8e099.

@FRidh FRidh mentioned this pull request Jun 26, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@talyz talyz talyz approved these changes

@aanderse aanderse aanderse approved these changes

@etu etu Awaiting requested review from etu

+3 more reviewers

@jhthorsen jhthorsen jhthorsen left review comments

@stigok stigok stigok approved these changes

@fnords fnords fnords approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@stigtsp @talyz @FRidh @jhthorsen @stigok @fnords @aanderse