Ticket #4170 (closed enhancement: fixed)

Opened 4 years ago

Last modified 7 weeks ago

Move CI from Travis to GitHub Actions

Reported by: zaytsev Owned by: zaytsev
Priority: critical Milestone: 4.8.33
Component: adm Version: master
Keywords: Cc:
Blocked By: #4584 Blocking: #3738, #4602
Branch state: merged Votes for changeset: committed-master

Attachments

Screenshot 2024-10-20 at 20.13.34.png (622.1 KB) - added by zaytsev 2 months ago.

Change History

comment:1 Changed 3 years ago by zaytsev

  • Milestone changed from 4.8.27 to 4.8.28

Update: I have migrated the CI from travis-ci.org to travis-ci.com - unfortunately, the 10k credits didn't last long. We have got a temporary grant of credits from Travis - hopefully this will last until I find time to migrate to GitHub? Actions.

comment:2 Changed 3 years ago by zaytsev

  • Milestone changed from 4.8.28 to 4.8.29

comment:3 Changed 2 years ago by zaytsev

  • Milestone changed from 4.8.29 to 4.8.30

First step is done with the Transifex workflows...

comment:4 Changed 2 years ago by zaytsev

  • Blocking 3738 added

(In #3738) Is only possible to implement after migrating to GitHub? Actions.

comment:5 Changed 16 months ago by zaytsev

  • Milestone changed from 4.8.30 to 4.8.31

Resurrected Travis first...

comment:6 Changed 11 months ago by zaytsev

  • Milestone changed from 4.8.31 to 4.8.32

Still too much work, sadly :(

comment:7 Changed 11 months ago by andrew_b

  • Milestone changed from 4.8.32 to Future Releases

comment:8 Changed 7 months ago by zaytsev

  • Owner set to zaytsev
  • Status changed from new to accepted
  • Milestone changed from Future Releases to 4.8.32

comment:9 Changed 4 months ago by zaytsev

  • Milestone changed from 4.8.32 to 4.8.33

comment:10 Changed 3 months ago by zaytsev

  • Blocked By 4584 added

comment:11 Changed 2 months ago by zaytsev

Oh man, on Fedora in Docker the tests are still failing:

Running suite(s): lib/mcconfig
Running suite lib/mcconfig
../../../../tests/lib/mcconfig/config_string.c:169:P:Core:test_create_ini_file_paths:0: Passed
../../../../tests/lib/mcconfig/config_string.c:169:P:Core:test_create_ini_file_paths:1: Passed
**
ERROR:../../../../tests/lib/mcconfig/config_string.c:196:test_create_ini_file_paths_fn: assertion failed (actual_value == data->expected_value): ("not-exists" == " \t \364\305\323\324\317\327\317\305 \332\316\301\336\305\316\311\305 ")
Bail out! ERROR:../../../../tests/lib/mcconfig/config_string.c:196:test_create_ini_file_paths_fn: assertion failed (actual_value == data->expected_value): ("not-exists" == " \t \364\305\323\324\317\327\317\305 \332\316\301\336\305\316\311\305 ")
../../../../tests/lib/mcconfig/config_string.c:169:E:Core:test_create_ini_file_paths:2: (after this point) Received signal 6 (Aborted)

It seems that we'll never get it done :(

comment:12 follow-up: ↓ 13 Changed 2 months ago by zaytsev

I think I've managed to add CI on:

  • Ubuntu (main system)
  • macOS
  • Fedora
  • Alpine

https://github.com/MidnightCommander/mc/actions/runs/11417299454

Andrew, any wishes?

I'm thinking Solaris would be great if possible. Other systems like BSD would be interesting, but I don't have any experience, and would like to add CI as soon as possible, actually, so that you can check for warnings, and see if things build at all for platforms which we managed to bring into supported status... So I would rather postpone anything beyond that.

comment:13 in reply to: ↑ 12 Changed 2 months ago by andrew_b

Replying to zaytsev:

Andrew, any wishes?

Nothing.

comment:14 Changed 2 months ago by andrew_b

Branch: 4170_github_ci

Version 0, edited 2 months ago by andrew_b (next)

comment:15 Changed 2 months ago by zaytsev

I think I've found a practical way to run Solaris on GitHub Actions, so I would also add that, because we've got so far with porting. I've also now have a real Solaris (very slow) on my laptop instead of Illumos. Unfortunately, it's not exactly the same as Illumos and there are still some problems to solve...

comment:16 Changed 2 months ago by zaytsev

  • Branch state changed from no branch to on rework

Okay, I think problems are solved, also on Solaris:

https://github.com/MidnightCommander/mc/actions/runs/11426161958

Still need to remove / re-implement Travis stuff.

comment:17 Changed 2 months ago by andrew_b

use `tail -1`, not `tail -n 1`

Why? Why not the same for head?

comment:18 follow-up: ↓ 20 Changed 2 months ago by zaytsev

  • Branch state changed from on rework to on review

I think I'm done - after a dozen of pushes, CI seems quite stable. Sources publishing is also re-implemented:

http://source.midnight-commander.org

Ready for review.

Why? Why not the same for head?

Because non-XPG4 version of tail fails if you call it as tail -n 1, but tail -1 works everywhere and is portable. However, non-XPG4 version of head accepts -n 1, apparently, because it was developed a bit later - so doesn't make sense to change it. Crazy, I know.

P.S. I can't thank mooffie enough for developing the extfs tests. Unfortunately, we don't have sample data for all scripts, but at least those that have sample data can be checked automatically now.

comment:19 Changed 2 months ago by zaytsev

Proof:

% ssh solaris                                        
Last login: Sun Oct 20 12:20:56 2024 from 192.168.64.1
Oracle Solaris 11.4.42.111.0                  Assembled December 2021
root@solaris:~# tail -n 1 .profile 
usage: tail [+/-[n][lbc][f]] [file]
       tail [+/-[n][l][r|f]] [file]
root@solaris:~# tail -1 .profile 

root@solaris:~# head -n 1 .profile 
#
root@solaris:~# head -1 .profile 
#     
root@solaris:~# /usr/xpg4/bin/tail -n 1 .profile 

comment:20 in reply to: ↑ 18 Changed 2 months ago by andrew_b

Replying to zaytsev:

Why? Why not the same for head?

Because non-XPG4 version of tail fails if you call it as tail -n 1, but tail -1 works everywhere and is portable.

I think it should be written in the commit message.

comment:21 follow-up: ↓ 22 Changed 2 months ago by andrew_b

Travis sends a notification via e-mail if building fails. Can Github do that?

comment:22 in reply to: ↑ 21 Changed 2 months ago by zaytsev

Replying to andrew_b:

Travis sends a notification via e-mail if building fails. Can Github do that?

Well, Travis doesn't do it anymore since March, because it's broken again for unknown reasons and I can't even log in to see why it's broken (did the credits run out again?), because the login is broken.

But yes, GitHub can do that. It sends a mail to the committer if the build is broken. See attached screenshot. You can also make it do any actions you want, of course - for that you'll have to write more YAML code.

I think it should be written in the commit message.

OK, I've amended the commit message.

Changed 2 months ago by zaytsev

comment:23 Changed 2 months ago by zaytsev

  • Blocking 4602 added

(In #4602) I think I saw some script to check for warnings. Maybe we could activate it in the CI.

comment:24 Changed 2 months ago by zaytsev

I'm added documentation listing to CI and fixed the warning.

comment:25 Changed 2 months ago by zaytsev

It seems that our hard work to fix tests and portability issues is finally paying off! I have just added FreeBSD to the CI, and didn't even have to fix anything, really:

https://github.com/MidnightCommander/mc/actions/runs/11442677163

And FreeBSD is an unexpectedly nice system to work with.

comment:26 Changed 7 weeks ago by zaytsev

Anything else to be done here (other than removing Travis from our organization)? I have rebased the branch again and cleaned up the commits.

I was able to --enable-werror almost everywhere expect for macOS because of the warnings coming from gnu tar stuff generated by clang. But at least we have now --enable-werror on clang from FreeBSD after all my fixes.

I think it would be good to merge this before any new branches are merged, so that they are tested.

comment:27 Changed 7 weeks ago by andrew_b

doc/man/pl/mc.1.in

-.I ftp://[!][użytkownik[:hasło]@]komputer[:port][zdalny katalog]
+.I ftp://[!][użytkownik[:hasło]@]komputer[:port]/[zdalny-katalog]
+.I ftp://[!][użytkownik[:hasło]@]komputer[:port]/[zdalny\-katalog]

comment:28 Changed 7 weeks ago by zaytsev

Added a fixup, thanks! Sadly groff doesn't warn about it and you can only find this out if you change local glyph mappings :(

comment:29 Changed 7 weeks ago by andrew_b

  • Votes for changeset set to andrew_b
  • Branch state changed from on review to approved

comment:30 Changed 7 weeks ago by zaytsev

  • Status changed from accepted to testing
  • Votes for changeset changed from andrew_b to committed-master
  • Resolution set to fixed
  • Branch state changed from approved to merged

Merged to master as [494aeabe274cb6a64cd44241402c42111c153a47].

git log --oneline 4bd73a7de..494aeabe2

Fixed Transifex paths in 9e4e7e8a7fd495d0cb83166a0542736e8dc10ebf .

Last edited 7 weeks ago by andrew_b (previous) (diff)

comment:31 Changed 7 weeks ago by zaytsev

  • Status changed from testing to closed
Note: See TracTickets for help on using tickets.