darcs

Patch 2118 ci: include pending files in the git commit (and 21 more)

Title ci: include pending files in the git commit (and 21 more)
Superseder Nosy List bfrk
Related Issues
Status accepted Assigned To
Milestone

Created on 2020-11-19.18:20:10 by bfrk, last changed 2020-12-19.23:43:48 by ganesh.

Files
File name Status Uploaded Type Edit Remove
ci_-add-comments-and-slightly-extend-the-trigger_ci-script.dpatch bfrk, 2020-11-21.15:57:26 application/x-darcs-patch
ci_-enable-running-the-tests-on-windows.dpatch bfrk, 2020-11-20.15:50:03 application/x-darcs-patch
ci_-include-pending-files-in-the-git-commit.dpatch bfrk, 2020-11-19.18:20:08 application/x-darcs-patch
patch-preview.txt bfrk, 2020-11-19.18:20:08 text/x-darcs-patch
patch-preview.txt bfrk, 2020-11-20.15:50:03 text/x-darcs-patch
patch-preview.txt bfrk, 2020-11-21.15:57:25 text/x-darcs-patch
unnamed bfrk, 2020-11-19.18:20:08 text/plain
unnamed bfrk, 2020-11-20.15:50:03 text/plain
unnamed bfrk, 2020-11-21.15:57:26 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg22515 (view) Author: bfrk Date: 2020-11-19.18:20:08
This finally brings the CI on Windows on par with the other systems. All
tests that were previously enabled on Windows succeed, plus a few more.

Caching of the cabal store seems to work pretty reliably. However, cabal
update (necessary after potential installation of cabal) may result in cabal
picking some newer dependencies, and the cabal index is not (yet?) part of
the cache key.

22 patches for repository http://darcs.net/screened:

patch 785e640f5ccebadb56b91c3e8b43a4e177ff40bb
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Nov 15 21:50:30 CET 2020
  * ci: include pending files in the git commit

patch a7a541e223c791219cf75b1f996445675a8cee52
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Nov 15 22:05:52 CET 2020
  * tests: more of the usual test repo removal fixes

patch 2117d63a0d309910439ceb550fb3a89f049a9de4
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Nov 15 05:10:39 CET 2020
  * tests: simplify comparing patch bundles

patch 63a210cdc6f362f0846b9a84628a56144a15c11a
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Nov 15 23:17:28 CET 2020
  * tests: compare paths in a more portable fashion
  
  This replaces the grep pattern '/filename$' with uses of test and basename,
  which has a better chance of succeeding on Windows.

patch f2320a7c3bfe7a3a475ea33a22abaf8dbe4373b4
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Nov 15 23:23:52 CET 2020
  * fix broken tests/issue1101.sh

patch a32119ed881e3ad47adc0bc2802c58d76826bdce
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Nov 15 23:34:28 CET 2020
  * ci: re-configure git globally on windows
  
  This must be done to prevent our test data from being checked out with
  additional CRs.

patch 3b47f27c22153a70c9b100e75a0c987141cd1abc
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Nov 17 10:16:05 CET 2020
  * remove unused function resendEmail

patch b422470a04a1faed3449bc5fe12830a463636bca
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Nov 16 09:35:42 CET 2020
  * clean up some of the email sending mess
  
  Searching for a sendmail command in paths like /sbin makes sense only on
  Unix-like systems. This code is now guarded behind #ifndef WIN32. Since we
  always use MAPI on Windows, unless a sendmail-command is specified via the
  option or the SENDMAIL environment, I have replaced HAVE_MAPI with WIN32
  enerywhere. Furthermore, the send command itself no longer has #ifdefs, as
  we pull the check for early failure into D.UI.External as (Os-independent)
  checkDefaultSendmail, adding a TODO comment that this should be improved (on
  Windows) to check whether using MAPI has a chance to succeed.

patch cf52f0609c2b842a2602977f1318d286d12ba8db
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Nov 19 08:53:57 CET 2020
  * add a debug message to execSendmail

patch 6254775aee8756f383bd7f2c2073947e1f35eff3
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Nov 19 11:04:45 CET 2020
  * more Windows test fixes

patch 5548e1e449deadb4d45e011dbf57faea54265f45
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Nov 19 11:01:18 CET 2020
  * fix tests/send-external.sh so that it works on windows, too
  
  Unfortunately it is not fully clear why invoking the generated script via
  bash does not work on Windows, whereas invoking a Haskell script via runghc
  works.

patch 7abf2058b2aafca5dc9f8cdb6979e6dc26b28018
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Nov 15 12:04:45 CET 2020
  * tests/add_permissions.sh: enable commented parts of the test

patch a84c0be7b93858fa34e66a50bea6662023a6ed75
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Nov 15 23:20:29 CET 2020
  * tests/lib: include \r in IFS on Windows
  
  This makes several of the tests succeed when I run the CI.

patch 0ab43c70d561e9c20e9e47cb2ed984a8a83cf8b7
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Nov 15 22:06:37 CET 2020
  * tests/lib: factor out os_is_windows function

patch 537b0fa68d9c43de1a02907641b4a01de685fe9b
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Nov 19 17:12:34 CET 2020
  * ci: use ghc-8.8.4, not ghc-8.8.2

patch 4402e3fe0f7771fa3957ab338c5989fcfb97a385
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Nov 19 17:13:05 CET 2020
  * ci: remove --hidesuccesses when running the tests
  
  It turned out that when we accidentally invoke MAPI on Windows, the command
  may hang indefinitely. With --hidesuccesses it is hard to detect that
  situation since running the tests on Windows is very slow.

patch eb671eabdba29306567e679e00f22d30e4f25d46
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Nov 19 17:16:48 CET 2020
  * ci: add ACTIONS_ALLOW_UNSECURE_COMMANDS
  
  This became necessary because of a change in the github actions. Hopefully
  actions/setup-haskell will be adapted soon so we can take this out again.

patch 7a73bbb399cc3870f4abe73485d4d83cec26e593
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Nov 19 17:19:35 CET 2020
  * ci: use runner.os instead of fiddling with matrix.os
  
  The runner.os gives us the OS class in a simple way.

patch 187003b62f8a23560c4cb9a9674cd9af6432431c
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Nov 19 17:20:36 CET 2020
  * ci: restructure the caching apparatus

patch 866ad295ce8db5a1392234cd3dcf0e7649411827
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Nov 16 10:02:45 CET 2020
  * tests: remove obsolete setting of DARCS_EDITOR
  
  We nowadays have --no-edit-description in the defaults when testing.

patch 70d3b5f1e058e4c37b6f45956e6a1216c78fd0b2
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Nov 19 11:06:16 CET 2020
  * tests/printer.sh: disable test for \r encoding on Windows
  
  This fails for reasons that are still unclear to me.

patch 7828caac15f2e9f195a9881ebd8b7e5274719f4d
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Nov 19 17:21:54 CET 2020
  * ci: enable running the tests on Windows
Attachments
msg22518 (view) Author: bfrk Date: 2020-11-20.15:50:03
Here is one more patch to make testing on Windows a bit more practical. (The
other one is just a dependency.)

2 patches for repository http://darcs.net/screened:

patch 7828caac15f2e9f195a9881ebd8b7e5274719f4d
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Nov 19 17:21:54 CET 2020
  * ci: enable running the tests on Windows

patch df9a12da689439e9ae0e131340339562dec2391a
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Nov 20 16:58:12 CET 2020
  * ci: reduce tests on Windows to darcs-2 format
  
  Running the shell tests on Windows takes about thrice as long as for Linux
  and macOS, over one hour in total.
Attachments
msg22524 (view) Author: ganesh Date: 2020-11-21.10:38:43
I confirm the tests pass on my Windows machine with this bundle. It's
fine to screen.

Doing a full review now of everything except the sendmail/MAPI patch
which I want to take a slightly closer look at.

>   * ci: include pending files in the git commit

It might help to clarify in trigger-ci that it is testing the full
unrecorded state, not just the recorded state. Given that, using
--pending is natural (though the "darcs show files" docs suggest it
should be the default anyway?)

>   * tests: more of the usual test repo removal fixes

OK

>   * tests: simplify comparing patch bundles

Nice.


>   * tests: compare paths in a more portable fashion

OK

>   * fix broken tests/issue1101.sh

OK

>   * ci: re-configure git globally on windows

OK

>   * remove unused function resendEmail

OK


>   * clean up some of the email sending mess

TODO

>   * add a debug message to execSendmail

OK

>   * more Windows test fixes

OK

>   * fix tests/send-external.sh so that it works on windows, too
>   
>   Unfortunately it is not fully clear why invoking the generated script via
>   bash does not work on Windows, whereas invoking a Haskell script via runghc
>   works.

OK - I don't think invoking external scripts via bash has ever worked on
Windows, which is why there are typically Haskell helpers for the job. I
can't remember exactly why though and I agree in principle it seems like
it ought to work as people on Windows running the tests do have bash.

>   * tests/add_permissions.sh: enable commented parts of the test

OK


> patch a84c0be7b93858fa34e66a50bea6662023a6ed75
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Sun Nov 15 23:20:29 CET 2020
>   * tests/lib: include \r in IFS on Windows
>   
>   This makes several of the tests succeed when I run the CI.

> +  git config --global core.autocrlf input

Did you intend to include this line there too? If so I think it could do
with a comment as it's a bit weird for our core code/tests to be
invoking git.


>   * tests/lib: factor out os_is_windows function

OK

>   * ci: use ghc-8.8.4, not ghc-8.8.2

OK

>   * ci: remove --hidesuccesses when running the tests
>   
>   It turned out that when we accidentally invoke MAPI on Windows, the command
>   may hang indefinitely. With --hidesuccesses it is hard to detect that
>   situation since running the tests on Windows is very slow.

OK

>   * ci: add ACTIONS_ALLOW_UNSECURE_COMMANDS
OK

>   * ci: use runner.os instead of fiddling with matrix.os

OK

> 
>   * ci: restructure the caching apparatus

OK

>   * tests: remove obsolete setting of DARCS_EDITOR

OK

>   * tests/printer.sh: disable test for \r encoding on Windows
>   
>   This fails for reasons that are still unclear to me.

The comment could more accurately say "This doesn't seem to work on
Windows in CI" as it does work for me, but not a big deal.


>   * ci: enable running the tests on Windows

OK
msg22527 (view) Author: bfrk Date: 2020-11-21.15:57:26
Following up on review.

2 patches for repository http://darcs.net/screened:

patch 700f7397ab87a3a15cd3022289f0eb84cb6bf57a
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Nov 21 16:48:00 CET 2020
  * ci: add comments and slightly extend the trigger-ci script
  
  The comment explains that what is committed into the git repo is the pending
  unrecorded state, not the recorded state. Also remove the redundant
  --pending flag for darcs show files. Instead, add -A flag to 'git add' in
  order to include pending file removals with the commit.

patch 848145778a8a8b6f8f6f1be9c61e52df2ca39e5b
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Nov 21 17:04:53 CET 2020
  * tests/lib: add comments to explain special Windows settings
Attachments
msg22560 (view) Author: ganesh Date: 2020-12-19.20:33:42
Reviewing followups (sorry for the dupe, sending this to the right place
this time):

> patch 700f7397ab87a3a15cd3022289f0eb84cb6bf57a
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Sat Nov 21 16:48:00 CET 2020
>   * ci: add comments and slightly extend the trigger-ci script
>
>   The comment explains that what is committed into the git repo is the
>   pending
>   unrecorded state, not the recorded state. Also remove the redundant
>   --pending flag for darcs show files. Instead, add -A flag to 'git
>   add' in
>   order to include pending file removals with the commit.

OK

> patch 848145778a8a8b6f8f6f1be9c61e52df2ca39e5b
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Sat Nov 21 17:04:53 CET 2020
>   * tests/lib: add comments to explain special Windows settings

OK
msg22572 (view) Author: ganesh Date: 2020-12-19.23:15:30
I've looked at the MAPI patches too now and I'm fine with them, though
I haven't tried out using it myself yet.
History
Date User Action Args
2020-11-19 18:20:10bfrkcreate
2020-11-20 15:50:10bfrksetfiles: + patch-preview.txt, ci_-enable-running-the-tests-on-windows.dpatch, unnamed
messages: + msg22518
2020-11-21 10:38:46ganeshsetmessages: + msg22524
2020-11-21 15:55:39bfrksetstatus: needs-screening -> needs-review
2020-11-21 15:57:27bfrksetfiles: + patch-preview.txt, ci_-add-comments-and-slightly-extend-the-trigger_ci-script.dpatch, unnamed
messages: + msg22527
2020-11-21 15:58:11bfrksetstatus: needs-review -> review-in-progress
2020-12-19 20:33:43ganeshsetmessages: + msg22560
2020-12-19 23:15:31ganeshsetstatus: review-in-progress -> accepted-pending-tests
messages: + msg22572
2020-12-19 23:43:48ganeshsetstatus: accepted-pending-tests -> accepted