-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
[RFC] Add better way to install travis dependencies #350
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
Conversation
All dependencies for building/testing are built into an external github repository. This script will install everything into /opt/neovim-deps, and by having github as single point of failure, travis builds are now very reliable.
Will the repo move to the neovim organisation? Even though you are the founder/leader/dictator of the project it is confusing to have it under your account while the rest is under the neovim organisation. |
If @jszakmeister approves this solution, then yes, I will move the repo. |
Honestly, even though it tickles my "you've just checked binaries into a repository" alarm, it's a useful idea. Speeding up the Travis build would be nice. It's dog slow right now. :-( |
@jszakmeister another option would be to use a ppa to build and install the packages. We are already using one to install clang 3.4. |
Having faster travis runs is great! But how will travis know where to find the binaries? Or is the commit for exporting the new $PATH still forthcoming? Or are you going for explicit usage of /opt/../bin/luajit calls? |
@tarruda I had considered that, but unfortunately, I've not setup a PPA before. I'm not sure what's required. What you've done is more straight-forward. Though having a PPA might help others more, especially if it's built for several Ubuntu variants. @aktau It's not hard to search other places. |
I have no experience with ppa either. From what I could see in the help page, it is simple for someone who knows how to create debian source packages. Perhaps for now we can use the github approach, at least until some debian expert steps forward to make this contribution. |
Making source packages is actually not extremely hard if you don't care about following every rule of the debian maintainers guide (which is crazy). There's some really nice tools to help:
|
Just wanted to comment that I've been looking at packaging the neovim dependencies. I'd like to host it off launchpad, if at all possible. It would help to get deps generated for more Ubuntu platforms if we could. Sorry it's taking a while to get it sorted out though. |
@jszakmeister thats great, it will greatly improve travis build time :). I wonder if its possible to host the package source at github instead of launchpad. Do you know ubuntu ppas are github-friendly?. As @aktau suggested, there's no need to follow debian packaging rules for this, just bundle everything into one big package. |
AFAIK, no, they're not. But, I could be mistaken. To put them up on launchpad, I need to create a src deb. I'm not looking to be compliant, but I don't think it's as easy as using fpm and tossing it up there--especially if we want to have the deps build for newer Ubuntu releases. I'm trying to work out what the minimum necessary bits are, but the docs leave a little to be desired. I'm hoping to hit up a local Ubuntu group and see if they have some information though. |
@jszakmeister luarocks started failing again(#540) so I'm gonna update this PR and use it a temporary solution until the deb src is ready. |
Temporarily fixed by #550 |
Recent luarocks adds support for these env vars: http_proxy https_proxy no_proxy Closes neovim#2482 Changes since previous luarocks version (27 Aug 2014): git log --oneline 0587afbb5fe8ceb2f2eea16f486bd6183bf02f29..HEAD 5d8a165 Merge pull request neovim#371 from ignacio/proxies 4462ca5 Add `luarocks config` command for querying LuaRocks settings. b80244b Merge branch 'master' of https://github.com/keplerproject/luarocks dd6f0e7 Update lmathx used for testing Lua 5.3 70c7577 Merge pull request neovim#366 from Tieske/windows_exitcode 11b8b48 fixes neovim#365 0d071fa Back to scm 7bff020 Mark release 2.2.2 2f9c115 Merge branch 'master' of https://github.com/keplerproject/luarocks 9736020 Install .md files as docs 97b98bf Clip string.gsub results to just one when redacting url. 2a0a9fa Merge pull request neovim#359 from ignacio/redact_verbose 968e963 Redact api tokens when using --verbose flag 9aa5d05 Update upload URL as well. cda43ce Merge branch 'master' of https://github.com/keplerproject/luarocks 022c87d MoonRocks → LuaRocks.org transition complete! 7b6efb9 Trust the user :) 2c536b4 Deal with 'no_proxy' env var f022fe0 Drop use of config.proxy b6b6754 Merge pull request neovim#354 from Tieske/pe_parser 460e42d update to version 0.3 2ee6bd7 Merge pull request neovim#349 from ignacio/build_only_deps 15ad97b Address issues spotted in the review 6b350de Adds --only-deps flag to install command 6dd402b Adds new file (fetch/git_https.lua) to Makefile.setup.inc 46f8ad6 Merge pull request neovim#350 from jszakmeister/add-git-https-support 989347e Add git+https support. 0f67be5 Adds --only-deps flag to the 'build' command. 0fe8556 Update function documentation, as suggested by @ignacio in neovim#347. 0679559 Decided to step back in turn this into a warning. `luarocks list` on an empty ~/.luarocks is a valid use case. 40f9173 Fail when given an invalid tree. 6d5dfcd Fix crash on `luarocks --tree=/path list`. Closes neovim#347. 1fcf354 Add test that checks for error in default sysconfig. See neovim#346. 3ce554c Restore comment about second return, but put it in the right function. luarocks/luarocks#346 (comment) 0e3a052 Merge pull request neovim#346 from Tieske/bad_config c66a88e bail out on bad config files, fixes neovim#228 79addc7 Continuing slowly. Distracted by code golf. :) 58fb6b9 Merge branch 'master' of https://github.com/keplerproject/luarocks ed1f916 Starting to port test suite from Unix shell to Lua. 6f87c47 Merge pull request neovim#343 from xpol/master cbde573 And also hide the startup logo for RC. 5cb4aa7 Merge branch 'master' of https://github.com/keplerproject/luarocks 303cca7 Add AppVeyor badge ad8ba47 Merge pull request neovim#335 from ignacio/appveyor a52b5ca Merge branch 'master' of https://github.com/keplerproject/luarocks 6251735 Add Coveralls coverage badge 2fcc0cc Add options to hide the MSVC tools' startup logo. ff68e97 Fallback for platform variable e31c46b Improved the CI scripts 050d656 Fix summary detection in long paragraphs 4ad1f1a Remove failing test. Try this some other time. db81c2e Force package to be in cache. 303628a Add more simple tests. 8d6a9e3 Merge branch 'master' of https://github.com/keplerproject/luarocks 5b45de2 More small tests. 066cda4 Merge pull request neovim#341 from keplerproject/add-travis 2639401 Make localhost a known host. a549c6d Try not to block checking server identification. 7c8e527 Let's see if Travis allow sftp'ing to localhost. 98e0979 Merge branch 'master' of https://github.com/keplerproject/luarocks 5f293dd Remove debugging print. ed02691 Add trivial tests for `luarocks upload` b4ea2a1 Merge pull request neovim#340 from xpol/master b9789f3 Revert incorrect remove of cmake_generator support. Only windows (msvc) default cmake_generator are removed. a19af6d luacov-coveralls overwrites luacov.report.out! 1b5bbfc luacov-coveralls did not exit with 0? df08baf Run luacov-coveralls from $testing_dir f3aaee7 Avoid tests that mess with the testing environment. 836898f Let's try Coveralls b5244be Merge branch 'master' of https://github.com/keplerproject/luarocks 30430cf Don't overwrite --detailed when given by the user. 19ca56c Actually direct users to the bug tracker 57c838e Merge branch 'master' of https://github.com/keplerproject/luarocks 5495f3c A missing CWD returns "" for lfs.current_dir() on Ubuntu db90cb4 Really test for missing parameters. d3d74bf A missing CWD returns "" for fs.current_dir on Ubuntu... a027595 Let's try harder to fail if CWD does not exist. 876d9c8 Fix inconsistency in --homepage flag in `luarocks doc` and `luarocks write_rockspec`. 294e08f Fix --lib flag (and my last commit goof...) 62d4e05 Fix tests: new flag parser detected invalid flags in the testsuite. 7f7c006 Add support for space in long option assignments. 68aa7ae Merge branch 'master' of https://github.com/keplerproject/luarocks e869c09 Fail nicely if CWD does not exist. Fixes neovim#147. ae51a3c Fix confusing error when unpack fails due to network error 93cdd54 Adds integration with AppVeyor 28ade76 Fixes neovim#332. 51ea074 Expose platform and processor to home config files. a02a53a Merge branch 'master' of https://github.com/keplerproject/luarocks 4c96972 Don't use user tree when running as root. Fixes neovim#303. f15e49d Merge pull request neovim#330 from mpeterv/hg-support 9567ac5 Merge pull request neovim#329 from mpeterv/persist-refactor 20eb947 Improve hg support cf19178 Refactor persist.save_from_table 3c7c472 Refactor persist.load_into_table 603b0ea Merge branch 'master' of https://github.com/keplerproject/luarocks be3c52d Add extra smartness to configure to check that the user-given flag seems correct. Closes neovim#293. d820069 Merge pull request neovim#326 from mpeterv/fix-redact-api 8739847 Merge branch 'master' of https://github.com/keplerproject/luarocks 5db7c54 Merge branch 'xpol-master' 7d22ee5 Open file in 'rb 90586f6 Merge branch 'master' of https://github.com/keplerproject/luarocks bdf218b Remove commented code after remove cfg.cmake_generator. b5e2539 Better cmake support. df332f6 Fix url redacting when Luasocket is used 88a903a Add logo :) 6e21673 Try the one we have as `lua` first! 4e9a0e3 This is for Makefile.luarocks only. ccab32f Merge branch 'new-makefile' 855259b New set of Makefiles for self-upgrade. ff6fdfc Ignore more files. 92d6363 Make sure suffix is produced when installing via rock (see neovim#323) and copy over site_config.lua, in case we're installing to a different prefix (see https://sourceforge.net/p/luarocks/mailman/message/33608257/) dc5f200 Make it a bit more robust. 4347dc7 Redact API URL to hide API key. 650c8ae Back to our regularly scheduled programming 8649a4e Release LuaRocks 2.2.1 c7a704a Add test files that were not committed before. 463ee89 Don't crash when modules table is missing. d110857 Use the system-installed stat. 0f9d259 Test success of patching in `unpack`. Closes neovim#316. Includes test cases for the test suite! Yay! 9a9caf8 We're always using the internal patch module. See neovim#316. c9cc478 All 5.x versions of Lua share the same license. 92c7acb Clarify that runtime support is optional. 5f3d390 Don't crash when asking for help on invalid cmd. 46f2d25 Code cleanups suggested by luacheck. 7fe62f1 Remove unused assignment. 53e0c65 Direct users to the bug tracker 2013547 Support both --lua-version and --with-lua-version. Error messages were even already using it by accident! 48847a4 Support more file extensions as source files. 23afae6 Merge branch 'master' of https://github.com/keplerproject/luarocks c54cbfc Fix behavior of `luarocks pack` on Windows. It was failing when a path contained spaces due to lack of quoting. Closes neovim#308. 7f6320c Merge pull request neovim#309 from mpeterv/unused_variables 500741f Removed some unused and global variables 113ada0 Merge branch 'master' of https://github.com/keplerproject/luarocks 9204178 Discard excess characters when a tool gives out an octal mode string that's too long. Fixes neovim#53. aa4e0d3 Merge pull request neovim#298 from seclorum/master 9702239 Use updated LuaFileSystem for Lua 5.3 0f1c937 Updates for Lua 5.3 compatibility 8d6845e Make conversion more robust for Lua 5.3 d98c3e0 Make it more robust. (I _think_ win32 needs something similar, but there's the complication of drive letters so I won't touch it now without proper testing.) 8d588f9 Catch error if filename is a directory 1885a7f Improve error checking f74346e Do not pack scm versions cd99315 Fix search of lua interpreter. Closes neovim#301. 4c503eb Update stdlib for 5.3 (thought I had this in the previous commit!) c5501d4 Merge branch 'master' of https://github.com/keplerproject/luarocks de654b3 Updates for Lua 5.3 support 4636244 use cprint version compatible with Lua 5.3 fc6d30d Update stdlib for Lua 5.3 compatibility 76e5515 Add Lua 5.3 to the test matrix 9ab9988 Add test that catches neovim#228. 0ebdcd4 Updates to testing infrastructure (use new luasec, luacov) e7f9680 Error out on bad config files. Alternative implementation to the one given by @Tieske, following discussion in neovim#260. Closes neovim#260. Closes neovim#228. 02e8bbd Safer guards for OSX Deployment target selection.. c4558a3 OSX 10.10 Yosemite sw_vers update db46b22 Apply change suggested by @siffiejoe. Thanks @catwell for catching this! Closes neovim#295. 1a1c407 Add test for neovim#295. 8bbf02e Make test suite detect crashes on tests that should fail gracefully. 7a7c124 Add check for Fedora systems. Closes neovim#289. 723bf99 Isolate the convenience hack, for readability. a35dd43 Silence complaints from `luarocks upload`. Closes neovim#292. af679a9 Fix typo. Closes neovim#294. 453179d Provide a fallback for when the version number is 'scm', to avoid breaking Windows default paths (which assume something like c:\luarocks\2.2\ ) Closes neovim#288. 88ea74e Make code more resilient. 0467eba Merge branch 'master' of https://github.com/keplerproject/luarocks 8278ed2 Add flag to enable/disable SSL cert check. We disabled SSL certificate checks for wget and curl a while ago, when we first added https repositories. We'll keep the check disabled by default for now, but this adds a config option, `check_certificates=true` that can be used in your config.lua. af19063 Don't report WIP versions as releases. d15e99f Merge pull request neovim#285 from mpeterv/fix-lint 86ba23c Fix `luarocks lint`. e5cd7a9 Add --outdated as a flag to `luarocks list`. A variation of the feature suggested in neovim#282. f0d66ae Support per-field version checking. This will allow us to add fields and bump rockspec version numbers in a well-behaved manner.
Recent luarocks adds support for these env vars: http_proxy https_proxy no_proxy Closes #2482 Changes since previous luarocks version (27 Aug 2014): git log --oneline 0587afbb5fe8ceb2f2eea16f486bd6183bf02f29..HEAD 5d8a165 Merge pull request #371 from ignacio/proxies 4462ca5 Add `luarocks config` command for querying LuaRocks settings. b80244b Merge branch 'master' of https://github.com/keplerproject/luarocks dd6f0e7 Update lmathx used for testing Lua 5.3 70c7577 Merge pull request #366 from Tieske/windows_exitcode 11b8b48 fixes #365 0d071fa Back to scm 7bff020 Mark release 2.2.2 2f9c115 Merge branch 'master' of https://github.com/keplerproject/luarocks 9736020 Install .md files as docs 97b98bf Clip string.gsub results to just one when redacting url. 2a0a9fa Merge pull request #359 from ignacio/redact_verbose 968e963 Redact api tokens when using --verbose flag 9aa5d05 Update upload URL as well. cda43ce Merge branch 'master' of https://github.com/keplerproject/luarocks 022c87d MoonRocks → LuaRocks.org transition complete! 7b6efb9 Trust the user :) 2c536b4 Deal with 'no_proxy' env var f022fe0 Drop use of config.proxy b6b6754 Merge pull request #354 from Tieske/pe_parser 460e42d update to version 0.3 2ee6bd7 Merge pull request #349 from ignacio/build_only_deps 15ad97b Address issues spotted in the review 6b350de Adds --only-deps flag to install command 6dd402b Adds new file (fetch/git_https.lua) to Makefile.setup.inc 46f8ad6 Merge pull request #350 from jszakmeister/add-git-https-support 989347e Add git+https support. 0f67be5 Adds --only-deps flag to the 'build' command. 0fe8556 Update function documentation, as suggested by @ignacio in #347. 0679559 Decided to step back in turn this into a warning. `luarocks list` on an empty ~/.luarocks is a valid use case. 40f9173 Fail when given an invalid tree. 6d5dfcd Fix crash on `luarocks --tree=/path list`. Closes #347. 1fcf354 Add test that checks for error in default sysconfig. See #346. 3ce554c Restore comment about second return, but put it in the right function. luarocks/luarocks#346 (comment) 0e3a052 Merge pull request #346 from Tieske/bad_config c66a88e bail out on bad config files, fixes #228 79addc7 Continuing slowly. Distracted by code golf. :) 58fb6b9 Merge branch 'master' of https://github.com/keplerproject/luarocks ed1f916 Starting to port test suite from Unix shell to Lua. 6f87c47 Merge pull request #343 from xpol/master cbde573 And also hide the startup logo for RC. 5cb4aa7 Merge branch 'master' of https://github.com/keplerproject/luarocks 303cca7 Add AppVeyor badge ad8ba47 Merge pull request #335 from ignacio/appveyor a52b5ca Merge branch 'master' of https://github.com/keplerproject/luarocks 6251735 Add Coveralls coverage badge 2fcc0cc Add options to hide the MSVC tools' startup logo. ff68e97 Fallback for platform variable e31c46b Improved the CI scripts 050d656 Fix summary detection in long paragraphs 4ad1f1a Remove failing test. Try this some other time. db81c2e Force package to be in cache. 303628a Add more simple tests. 8d6a9e3 Merge branch 'master' of https://github.com/keplerproject/luarocks 5b45de2 More small tests. 066cda4 Merge pull request #341 from keplerproject/add-travis 2639401 Make localhost a known host. a549c6d Try not to block checking server identification. 7c8e527 Let's see if Travis allow sftp'ing to localhost. 98e0979 Merge branch 'master' of https://github.com/keplerproject/luarocks 5f293dd Remove debugging print. ed02691 Add trivial tests for `luarocks upload` b4ea2a1 Merge pull request #340 from xpol/master b9789f3 Revert incorrect remove of cmake_generator support. Only windows (msvc) default cmake_generator are removed. a19af6d luacov-coveralls overwrites luacov.report.out! 1b5bbfc luacov-coveralls did not exit with 0? df08baf Run luacov-coveralls from $testing_dir f3aaee7 Avoid tests that mess with the testing environment. 836898f Let's try Coveralls b5244be Merge branch 'master' of https://github.com/keplerproject/luarocks 30430cf Don't overwrite --detailed when given by the user. 19ca56c Actually direct users to the bug tracker 57c838e Merge branch 'master' of https://github.com/keplerproject/luarocks 5495f3c A missing CWD returns "" for lfs.current_dir() on Ubuntu db90cb4 Really test for missing parameters. d3d74bf A missing CWD returns "" for fs.current_dir on Ubuntu... a027595 Let's try harder to fail if CWD does not exist. 876d9c8 Fix inconsistency in --homepage flag in `luarocks doc` and `luarocks write_rockspec`. 294e08f Fix --lib flag (and my last commit goof...) 62d4e05 Fix tests: new flag parser detected invalid flags in the testsuite. 7f7c006 Add support for space in long option assignments. 68aa7ae Merge branch 'master' of https://github.com/keplerproject/luarocks e869c09 Fail nicely if CWD does not exist. Fixes #147. ae51a3c Fix confusing error when unpack fails due to network error 93cdd54 Adds integration with AppVeyor 28ade76 Fixes #332. 51ea074 Expose platform and processor to home config files. a02a53a Merge branch 'master' of https://github.com/keplerproject/luarocks 4c96972 Don't use user tree when running as root. Fixes #303. f15e49d Merge pull request #330 from mpeterv/hg-support 9567ac5 Merge pull request #329 from mpeterv/persist-refactor 20eb947 Improve hg support cf19178 Refactor persist.save_from_table 3c7c472 Refactor persist.load_into_table 603b0ea Merge branch 'master' of https://github.com/keplerproject/luarocks be3c52d Add extra smartness to configure to check that the user-given flag seems correct. Closes #293. d820069 Merge pull request #326 from mpeterv/fix-redact-api 8739847 Merge branch 'master' of https://github.com/keplerproject/luarocks 5db7c54 Merge branch 'xpol-master' 7d22ee5 Open file in 'rb 90586f6 Merge branch 'master' of https://github.com/keplerproject/luarocks bdf218b Remove commented code after remove cfg.cmake_generator. b5e2539 Better cmake support. df332f6 Fix url redacting when Luasocket is used 88a903a Add logo :) 6e21673 Try the one we have as `lua` first! 4e9a0e3 This is for Makefile.luarocks only. ccab32f Merge branch 'new-makefile' 855259b New set of Makefiles for self-upgrade. ff6fdfc Ignore more files. 92d6363 Make sure suffix is produced when installing via rock (see #323) and copy over site_config.lua, in case we're installing to a different prefix (see https://sourceforge.net/p/luarocks/mailman/message/33608257/) dc5f200 Make it a bit more robust. 4347dc7 Redact API URL to hide API key. 650c8ae Back to our regularly scheduled programming 8649a4e Release LuaRocks 2.2.1 c7a704a Add test files that were not committed before. 463ee89 Don't crash when modules table is missing. d110857 Use the system-installed stat. 0f9d259 Test success of patching in `unpack`. Closes #316. Includes test cases for the test suite! Yay! 9a9caf8 We're always using the internal patch module. See #316. c9cc478 All 5.x versions of Lua share the same license. 92c7acb Clarify that runtime support is optional. 5f3d390 Don't crash when asking for help on invalid cmd. 46f2d25 Code cleanups suggested by luacheck. 7fe62f1 Remove unused assignment. 53e0c65 Direct users to the bug tracker 2013547 Support both --lua-version and --with-lua-version. Error messages were even already using it by accident! 48847a4 Support more file extensions as source files. 23afae6 Merge branch 'master' of https://github.com/keplerproject/luarocks c54cbfc Fix behavior of `luarocks pack` on Windows. It was failing when a path contained spaces due to lack of quoting. Closes #308. 7f6320c Merge pull request #309 from mpeterv/unused_variables 500741f Removed some unused and global variables 113ada0 Merge branch 'master' of https://github.com/keplerproject/luarocks 9204178 Discard excess characters when a tool gives out an octal mode string that's too long. Fixes #53. aa4e0d3 Merge pull request #298 from seclorum/master 9702239 Use updated LuaFileSystem for Lua 5.3 0f1c937 Updates for Lua 5.3 compatibility 8d6845e Make conversion more robust for Lua 5.3 d98c3e0 Make it more robust. (I _think_ win32 needs something similar, but there's the complication of drive letters so I won't touch it now without proper testing.) 8d588f9 Catch error if filename is a directory 1885a7f Improve error checking f74346e Do not pack scm versions cd99315 Fix search of lua interpreter. Closes #301. 4c503eb Update stdlib for 5.3 (thought I had this in the previous commit!) c5501d4 Merge branch 'master' of https://github.com/keplerproject/luarocks de654b3 Updates for Lua 5.3 support 4636244 use cprint version compatible with Lua 5.3 fc6d30d Update stdlib for Lua 5.3 compatibility 76e5515 Add Lua 5.3 to the test matrix 9ab9988 Add test that catches #228. 0ebdcd4 Updates to testing infrastructure (use new luasec, luacov) e7f9680 Error out on bad config files. Alternative implementation to the one given by @Tieske, following discussion in #260. Closes #260. Closes #228. 02e8bbd Safer guards for OSX Deployment target selection.. c4558a3 OSX 10.10 Yosemite sw_vers update db46b22 Apply change suggested by @siffiejoe. Thanks @catwell for catching this! Closes #295. 1a1c407 Add test for #295. 8bbf02e Make test suite detect crashes on tests that should fail gracefully. 7a7c124 Add check for Fedora systems. Closes #289. 723bf99 Isolate the convenience hack, for readability. a35dd43 Silence complaints from `luarocks upload`. Closes #292. af679a9 Fix typo. Closes #294. 453179d Provide a fallback for when the version number is 'scm', to avoid breaking Windows default paths (which assume something like c:\luarocks\2.2\ ) Closes #288. 88ea74e Make code more resilient. 0467eba Merge branch 'master' of https://github.com/keplerproject/luarocks 8278ed2 Add flag to enable/disable SSL cert check. We disabled SSL certificate checks for wget and curl a while ago, when we first added https repositories. We'll keep the check disabled by default for now, but this adds a config option, `check_certificates=true` that can be used in your config.lua. af19063 Don't report WIP versions as releases. d15e99f Merge pull request #285 from mpeterv/fix-lint 86ba23c Fix `luarocks lint`. e5cd7a9 Add --outdated as a flag to `luarocks list`. A variation of the feature suggested in #282. f0d66ae Support per-field version checking. This will allow us to add fields and bump rockspec version numbers in a well-behaved manner.
This is a rather huge refactor. Fixes neovim#350 and fixes neovim#348. It removes 'winnr' from 'maker'. The behaviour changes so that the output is only processed by windows, but not buffers. This means that `Neomake | sp` will not open the location list, until you go to the previous window.
All dependencies for building/testing are built into an external github
repository. This script will install everything into /opt/neovim-deps, and by
having github as single point of failure, travis builds are now very reliable.
From my POV this is a good thing, because we can rest assured that travis builds won't fail due to broken external servers. It will also speed up travis builds.
@jszakmeister what are your thoughts? Since you are handling build-related issues I leave this to your decision. If you like this solution, feel free to bundle it with #348