Topic: Travis codechecking misbehaviour

1 — 10 of 15 Posts 1 2 next ››

Nordfriese Topic Opener Joined: 2017-01-17, 18:07 Posts: 2057 OS: Debian Testing Version: Latest master One Elder of Players Location: 0x55555d3a34c0	Posted at: 2020-01-30, 20:04 Can anybody make sense of this travis log? https://travis-ci.org/widelands/widelands/jobs/643951401 Formatting finished. ++++git status -s +++[[ -n M src/editor/ui_menus/scenario_tool_field_owner_options_menu.cc ]] +++echo 'Code not properly formatted. Please run: '\''./utils/fix_formatting.py'\''' Code not properly formatted. Please run: './utils/fix_formatting.py' +++echo 'Also, consider installing the githooks by running: '\''./install-githooks.sh'\''' Also, consider installing the githooks by running: './install-githooks.sh' +++git diff +++exit 1 The command "./.travis.sh formatting" exited with 1. I added `git diff` to travis.sh to see what exactly the issue corrected by clang-format is but it seems to result in an empty diff. Running clang-format locally on the offending file does not modify it at all... Top Quote

hessenfarmer Joined: 2014-12-11, 23:16 Posts: 2745 One Elder of Players Location: Bavaria	Posted at: 2020-01-30, 21:30 problem is that our format script works differently on different machines. for this it was automated in former times on our bunnybot. This doesn't work on travis currently as travis can't write its changes back to github. I believe we need to find a way to do exactly this: writing the changes back to github and commit them. until then you could only try to change the file arbitrarily and hope the next run on your local machine would be sufficient. BTW: with my last commit travis was happy with the branch but not with the merged master so it is really some kind of gambling. Top Quote

Nordfriese Topic Opener Joined: 2017-01-17, 18:07 Posts: 2057 OS: Debian Testing Version: Latest master One Elder of Players Location: 0x55555d3a34c0	Posted at: 2020-01-30, 21:49 Thanks for the explanation I normally format the files automatically with the git hook on my linux, but ATM I only have my windows machine for sporadic fixes available. Now I am really missing good old bunnybot... Perhaps we could hack together a script to let github run clang-format on PRs and master using this: https://help.github.com/en/actions/automating-your-workflow-with-github-actions/about-github-actions What I still don´t understand though is why travis reports the file as changed while simultanously outputting that `git diff` is empty... Top Quote

Nordfriese Topic Opener Joined: 2017-01-17, 18:07 Posts: 2057 OS: Debian Testing Version: Latest master One Elder of Players Location: 0x55555d3a34c0	Posted at: 2020-01-30, 22:11 Yes, I think actions are the way to go. I´ll try to hack together a script to make github automatically run fix_formatting whenever a branch is pushed so we won´t always have to worry about the one platform-dependant extra whitespace. Top Quote

Nordfriese Topic Opener Joined: 2017-01-17, 18:07 Posts: 2057 OS: Debian Testing Version: Latest master One Elder of Players Location: 0x55555d3a34c0	Posted at: 2020-01-31, 10:29 I succeeded in letting GitHub automatically run clang-format whenever I push to my widelands fork. Now I "only" have to extend the script to commit and push the changes back to the branch. The formatting takes about 4-5 minutes due to the need to set up python and clang-format, is this an acceptable delay? Edited: 2020-01-31, 12:52 Top Quote

hessenfarmer Joined: 2014-12-11, 23:16 Posts: 2745 One Elder of Players Location: Bavaria	Posted at: 2020-01-31, 10:55 Nordfriese wrote: I succeeded in letting GitHub automatically run clang-format whenever I push to my widelands fork. Now I "only" have to extend the script to commit and push the changes back to the branch. The formatting takes about 4-5 minutes due to the need to set up python and clang-format, is this an acceptable delay? It takes almost that long on my dev machine as well so this isn't too bad. We should only avoid pushing until it is finished. And we need tu pull before the next push. So this should probably only happen for Pull Requests instead of branches. Top Quote

Nordfriese Topic Opener Joined: 2017-01-17, 18:07 Posts: 2057 OS: Debian Testing Version: Latest master One Elder of Players Location: 0x55555d3a34c0	Posted at: 2020-01-31, 10:59 I think if you push while the formatting is running, a running job might be automatically cancelled. Still have to test whether this works. And we need tu pull before the next push. So this should probably only happen for Pull Requests instead of branches. Good point, will try to implement such a filter Top Quote

Nordfriese Topic Opener Joined: 2017-01-17, 18:07 Posts: 2057 OS: Debian Testing Version: Latest master One Elder of Players Location: 0x55555d3a34c0	Posted at: 2020-01-31, 17:07 Turns out that workflows are not automatically cancelled, so I added a check for remote updates in which the outdated workflow just exits with 1. There´s still a time window of a few seconds where this might be problematic, but it should then just result in a "branches have diverged" error and cancel in the "old" workflow while the "new" workflow succeeds. Pull requests are more problematic. Apparently (?) when you create or update a PR, the action is triggered on the base branch and not on the fork, and pushing between forks is forbidden for security reasons. Still need to figure out whether there´s a workaround... Top Quote

stonerl Joined: 2018-07-30, 00:03 Posts: 327 Tribe Member	Posted at: 2020-01-31, 17:52 I don't think that this is machine dependent. Here is the diff after running codechek: `@ -56,8 +56,9 @@ ScenarioToolFieldOwnerOptionsMenu::ScenarioToolFieldOwnerOptionsMenu( list_.add( (boost::format(_("Player %1$s (%2$s)")) % std::to_string(static_cast<int>(p)) % name) .str(), - p, g_gr->images().get( - Widelands::get_tribeinfo(eia().egbase().map().get_scenario_player_tribe(p)).icon), + p, + g_gr->images().get( + Widelands::get_tribeinfo(eia().egbase().map().get_scenario_player_tribe(p)).icon), sel == p, (boost::format(_("Claim fields for %s")) % name).str()); }` It is dependent on the clang-format version. I guess travis runs a newer version than you do. Here is the one I run: `clang-format version 9.0.0 (tags/google/stable/2019-05-14)` Top Quote

Nordfriese Topic Opener Joined: 2017-01-17, 18:07 Posts: 2057 OS: Debian Testing Version: Latest master One Elder of Players Location: 0x55555d3a34c0	Posted at: 2020-01-31, 18:10 Thanks for the diff But my current machine doesn´t find a newer clang-format than `clang-format version 3.8.0-2ubuntu4 (tags/RELEASE_380/final)`. So I think some automation would be nice anyway... I´ve found a way to check in my script for whether the branch has an open pull request, so my workflow is now starting after every push to every branch, and it cancels immediately unless it´s the `master` branch or a branch with open PR. Top Quote

1 2 next ››