Ethernet library outdated (and Travis CI formatting checks discussion)

[JAndrassy]

Hallo Hans. The included Ethernet library is old and shadows the current library. The Arduino Ethernet library now supports W5100, W520 and W5500 and has some new functions. It works over SPI library so it is multiplatform now and is not bundled with Arduino AVR core. I think it would be better to remove Ethernet library from MCUdude cores.

(I tested the ArduinoOTA library with MegaCore bootloader with OTEthernet example. Thank you for including the copy_flash_pages function. Did you test it with a 64 kB ATmega?)

[MCUdude]

Well, I do need to provide a dedicated library in order to be able to compile for the CAN32/64/128. This is because there is a variable in the library that collides with a CAN related register. About a year ago I updated the ethernet library that's bundled with MightyCore. I used Paul Stoffregen's Ethernet library. I'll update it for MegaCore as well.

I did include the copy_flash_pages functionality as an option when building the bootloader. However, it's not enabled in the pre-compiled binaries. To be honest, I haven't tested it yet. I probably should 😉

[MCUdude]

@per1234 it seems as the last Travis build fails due to tabs, trailing white spaces and missing EOL. To be honest it's very difficult to be able to fix all this in one commit without having Travis to check everything again and again. How do you usually deal with issues like this? If you have a trick up in your sleeve, would you like tu submit a PR to fix this? Thanks!

[per1234]

I've been thinking a lot lately on how to make it easy for developers to comply with the requirements imposed by Travis CI.

For simply detecting the issues locally, you can run the commands from .travis.yml (the "detection scripts" below).

For fixing up repositories in bulk when I first add a formatting check to its Travis CI build, I have a collection of scripts, which I have provided below ("Fix scripts").

Both the detection and fix scripts require bash. If using Windows, you can install Git for Windows, which includes Git Bash, and then run the scripts using Git Bash.

Git pre-commit hooks are run when you commit to a repository. If the script returns a non-0 exit status, the commit will be aborted. To install a Git pre-commit hook, create a file named pre-commit in .git/hooks, copy the script into it, and save. If you are using Linux (and probable macOS too), make the file executable (chmod +x). Note that hooks are installed on a per-repository basis. .git/hooks is not tracked by Git so you can't push your hooks to the remote repository to be installed for anyone cloning the repo.

I'm not super happy with the pre-commit hook system I'm using. I wanted the pre-commit hook to run a check identical to that done by Travis CI. That means the repository should be checked as it is at the point of the commit. Unstaged changes should not be checked. To accomplish this, I git stash before running the check, then git stash pop after. Untracked files are also skipped (which is no trouble). I'm not crazy about the hook automatically messing with the repository's index with the stash operations. The only alternative I've found is to to git checkout the repository to a temporary folder, run the check on that folder, and then delete the folder. That's not an ideal solution either, and is more complex since the user should be allowed to configure the location of the folder, but it may end up being preferable to the stash solution. The other option is to forget about the goal of skipping unstaged changes and run the check directly on the entire repository folder. I'd welcome any input on which is best, or any other possible solutions. Maybe @SpenceKonde would have some input or interest in this topic as well.

Let me know if you think it would be useful to add any of this to the repository. I now have a repository that is a collection of Git hook scripts. I'm planning to add the formatting check hooks below to it but I will need to remove the MCUdude *Core-specific paths from them. I'm planning to make it easy to configure the paths to skip via a configuration variable at the top of the script eventually but that means going down the ugly path of eval.

Non-Unix line endings

Detection script:

find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or -type f -exec grep --files-with-matches --binary-files=without-match --regexp=$'\r$' '{}' \; -exec echo 'Non-Unix EOL detected.' \; -exec false '{}' +

Git pre-commit hook:

#!/bin/bash

# stash so that unstaged changes are not checked
git stash -q --keep-index

# set default exit status
exitStatus=0

while read -r filename; do
  # check if the file is tracked by git
  if ! git ls-files --error-unmatch "$filename" &>/dev/null; then
    # don't check untracked files
    continue
  fi

  if grep --files-with-matches --binary-files=without-match --regexp=$'\r$' "$filename"; then
    echo "ERROR: Non-Unix EOL detected in $filename"
    # make the hook fail
    exitStatus=1
  fi
done <<<"$(find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or -type f)"

git stash pop -q

exit $exitStatus

Fix script:

# find all files under the current directory that have non-Unix EOL and convert them to Unix EOL
find . -path './.git' -prune -or -type f -exec grep --files-with-matches --binary-files=without-match --regexp=$'\r$' '{}' \; -exec echo 'Non-Unix EOL detected.' \; -exec dos2unix '{}' \; -exec false '{}' +
# Pause so that the output may be examined before script window is exited
read -rsp $'\n\nFinished converting files to Unix EOL. Press any key to continue...\n' -n 1

Trailing whitespace

Detection script:

find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or -type f -exec grep --with-filename --line-number --binary-files=without-match --regexp='[[:blank:]]$' '{}' \; -exec echo 'Trailing whitespace found.' \; -exec false '{}' +

Git pre-commit hook:

#!/bin/bash

# stash so that unstaged changes are not checked
git stash -q --keep-index

# set default exit status
exitStatus=0

while read -r filename; do
  # check if the file is tracked by git
  if ! git ls-files --error-unmatch "$filename" &>/dev/null; then
    # don't check untracked files
    continue
  fi

  if grep --with-filename --line-number --binary-files=without-match --regexp='[[:blank:]]$' "$filename"; then
    echo "ERROR: Trailing whitespace found in $filename"
    # make the hook fail
    exitStatus=1
  fi
done <<<"$(find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or -type f)"

git stash pop -q

exit $exitStatus

Alternate Git pre-commit hook (only checks staged changes):

#!/bin/bash

if git rev-parse --verify HEAD >/dev/null 2>&1
then
	against=HEAD
else
	# Initial commit: diff against an empty tree object
	against=$(git hash-object -t tree /dev/null)
fi

exec git diff-index --check --cached $against --

Fix script:

# Trim trailing whitespace from all files recursively
find . -path "./.git" -prune -or -type f -exec sed -i 's/[ \t]*$//' '{}' +

# Pause so that the output may be examined before script window is exited
read -rsp $'\n\nFinished trimming trailing whitespace. Press any key to continue...\n' -n 1

True tabs

Detection script:

find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or \( -not -name 'keywords.txt' -and -type f \) -exec grep --with-filename --line-number --binary-files=without-match --regexp=$'\t' '{}' \; -exec echo 'Tab found.' \; -exec false '{}' +

Git pre-commit hook:

#!/bin/bash

# stash so that unstaged changes are not checked
git stash -q --keep-index

# set default exit status
exitStatus=0

while read -r filename; do
  # check if the file is tracked by git
  if ! git ls-files --error-unmatch "$filename" &>/dev/null; then
    # don't check untracked files
    continue
  fi

  if grep --with-filename --line-number --binary-files=without-match --regexp=$'\t' "$filename"; then
    echo "ERROR: True tab found in $filename"
    # make the hook fail
    exitStatus=1
  fi
done <<<"$(find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or \( -not -name 'keywords.txt' -and -type f \))"

git stash pop -q

exit $exitStatus

Fix script:

I haven't written a fix script for replacing true tabs with spaces because different files may use different numbers of spaces for indentation. The true tabs must be replaced with the correct number of spaces to have the correct indentation after the fix.

Files starting with a blank line

Detection script:

find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or -type f -print0 | xargs -0 -L1 bash -c 'head -1 "$0" | grep --binary-files=without-match --regexp="^$"; if [[ "$?" == "0" ]]; then echo "Blank line found at start of $0."; false; fi'

Git pre-commit hook:

#!/bin/bash

# stash so that unstaged changes are not checked
git stash -q --keep-index

# set default exit status
exitStatus=0

while read -r filename; do
  # check if the file is tracked by git
  if ! git ls-files --error-unmatch "$filename" &>/dev/null; then
    # don't check untracked files
    continue
  fi

  if head -1 "$filename" | grep --binary-files=without-match --regexp="^$"; then
    echo "ERROR: Blank line found at start of $filename"
    # make the hook fail
    exitStatus=1
  fi
done <<<"$(find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or -type f)"

git stash pop -q

exit $exitStatus

Fix script:

#!/bin/bash

# Remove the first blank line from all files recursively

blankLineFound=true
while [[ $blankLineFound == true ]]; do
  find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or -type f -print0 | xargs -0 -L1 bash -c 'head -1 "$0" | grep --binary-files=without-match --regexp="^$"; if [[ "$?" == "0" ]]; then sed "1{/^[[:space:]]*$/d}" "$0" > "${0}.fixed"; mv --force "${0}.fixed" "$0"; false; else true; fi'
  if [[ $? == 0 ]]; then
    # No files start with a blank line so the process is done
    blankLineFound=false
  fi
done

# Pause so that the output may be examined before script window is exited
read -rsp $'\n\nFinished removing blank first lines. Press any key to continue...\n' -n 1

Extra blank lines at end of files

Detection script:

find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or -type f -print0 | xargs -0 -L1 bash -c 'tail -1 "$0" | grep --binary-files=without-match --regexp="^$"; if [[ "$?" == "0" ]]; then echo "Blank line found at end of $0."; false; fi'

Git pre-commit hook:

#!/bin/bash

# stash so that unstaged changes are not checked
git stash -q --keep-index

# set default exit status
exitStatus=0

while read -r filename; do
  # check if the file is tracked by git
  if ! git ls-files --error-unmatch "$filename" &>/dev/null; then
    # don't check untracked files
    continue
  fi

  if tail -1 "$filename" | grep --binary-files=without-match --regexp="^$"; then
    echo "ERROR: Blank line found at end of $filename"
    # make the hook fail
    exitStatus=1
  fi
done <<<"$(find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or -type f)"

git stash pop -q

exit $exitStatus

Fix script:

# Remove extra blank lines from the end of all files recursively
find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or -type f -print0 | xargs -0 -L1 bash -c 'tail -1 "$0" | grep --binary-files=without-match --regexp="^$"; if [[ "$?" == "0" ]]; then awk "/^$/ {nlstack=nlstack \"\n\"; next;} {printf \"%s\",nlstack; nlstack=\"\"; print;}" "$0" > "$0.fixed"; mv --force "$0.fixed" "$0"; echo fixing: "$0"; fi'

# Pause so that the output may be examined before script window is exited
read -rsp $'\n\nFinished removing extra blank lines from the end of files. Press any key to continue...\n' -n 1

Files that don't end in a newline

Detection script:

find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or -type f -print0 | xargs -0 -L1 bash -c 'if test "$(grep --files-with-matches --binary-files=without-match --max-count=1 --regexp='.*' "$0")" && test "$(tail --bytes=1 "$0")"; then echo "No new line at end of $0."; false; fi'

Git pre-commit hook:

#!/bin/bash

# stash so that unstaged changes are not checked
git stash -q --keep-index

# set default exit status
exitStatus=0

while read -r filename; do
  # check if the file is tracked by git
  if ! git ls-files --error-unmatch "$filename" &>/dev/null; then
    # don't check untracked files
    continue
  fi

  if test "$(grep --files-with-matches --binary-files=without-match --max-count=1 --regexp='.*' "$filename")" && test "$(tail --bytes=1 "$filename")"; then
    echo "ERROR: No new line at end of $filename"
    # make the hook fail
    exitStatus=1
  fi
done <<<"$(find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or -type f)"

git stash pop -q

exit $exitStatus

Fix script:

# Find all files that don't end in a newline recursively and add newline to the end of those files
find . -path './.git' -prune -or -type f -print0 | xargs -0 -L1 bash -c 'if test "$(grep --files-with-matches --binary-files=without-match --max-count=1 --regexp='.*' "$0")" && test "$(tail --bytes=1 "$0")"; then echo "No new line at end of $0."; echo >> "$0"; false; fi'
# Pause so that the output may be examined before script window is exited
read -rsp $'\n\nFinished adding missing newlines to end of files. Press any key to continue...\n' -n 1

Spell Check

Installing codespell:

1. Install Python 3: https://www.python.org/downloads/
2. pip install codespell

Detection script:

codespell --skip="./.git","./avr/bootloaders","./avr/cores","./avr/travis-ci" --ignore-words="./avr/travis-ci/etc/codespell-ignore-words-list.txt"

Git pre-commit hook:

#!/bin/bash

# check for commonly misspelled words
# codespell must be installed: https://github.com/codespell-project/codespell#installation

# configure the path to the codespell ignore words list file
# this list is used to fix false positives
# leave blank if there is no list file
readonly ignoreWordsListPath="./avr/travis-ci/etc/codespell-ignore-words-list.txt"

if [[ ! -e "$(command -v codespell)" ]]; then
  echo 'ERROR: codespell is not installed or not in PATH.'
  echo 'Please install it using: pip install codespell'
  exit 1
fi

# stash so that unstaged changes are not checked
git stash -q --keep-index

# set default exit status
exitStatus=0

while read -r filename; do
  # check if the file is tracked by git
  if ! git ls-files --error-unmatch "$filename" &>/dev/null; then
    # don't check untracked files
    continue
  fi

  if [[ "$ignoreWordsListPath" == "" ]]; then
    # there is no ignore words file so don't use the --ignore-words option
    if ! codespell "$filename"; then
      echo "ERROR: Misspelled word found in $filename"
      # make the hook fail
      exitStatus=1
    fi
  else
    if ! codespell --ignore-words="$ignoreWordsListPath" "$filename"; then
      echo "ERROR: Misspelled word found in $filename"
      # make the hook fail
      exitStatus=1
    fi
  fi
done <<<"$(find . -path './.git' -prune -or -path './avr/bootloaders' -prune -or -path './avr/cores' -prune -or -path './avr/travis-ci' -prune -type f)"

git stash pop -q

exit $exitStatus

Fix script:

codespell --write-changes --skip="./.git","./avr/bootloaders","./avr/cores","./avr/travis-ci" --ignore-words="./avr/travis-ci/etc/codespell-ignore-words-list.txt"

NOTE: codespell -w will only automatically fix misspelled words when there is only one suggested fix in the database. In the event there are multiple suggested fixes in the database, codespell will display the misspelled word, but not automatically fix it. You will need to manually fix the misspelled word.

[MCUdude]

Thank you so much for the scripts! As for now, it's easier for me to just manually run these detection scripts. I only really need them when I'm updating code from another repository. Do you know if there is a CRLF check as well? I prefer LF line endings only.

[per1234]

You're welcome. I'm glad if they can be of use. The "Non-Unix line endings" script checks for CRLF (Windows EOL) or CR (Macintosh EOL).

[MCUdude]

I forgot to check all files for extra blank lines before I pushed the code, so the current build will also fail. But when trying to run the script on my mac I get this output:

$ find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or -type f -print0 | xargs -0 -L1 bash -c 'if test "$(grep --files-with-matches --binary-files=without-match --max-count=1 --regexp='.*' "$0")" && test "$(tail --bytes=1 "$0")"; then echo "No new line at end of $0."; false; fi'
tail: illegal option -- -
usage: tail [-F | -f | -r] [-q] [-b # | -c # | -n #] [file ...]

It seems like the version bundled with macOS is a little bit different than on Ubuntu for instances. How can I run this on my mac?

EDIT:
Sorry, I'm mixing up the scripts. The script that checks for extra blank lines does work (you forgot to add it to the list above but I found it in the Travis output). The script above (check for no newline at end of file) doesn't though.

[per1234]

The error message is confusing. I guess it's the --bytes option that's not supported? I prefer to use long option names for scripts because it makes the command more self documenting, but I have noticed that sometimes the long option names aren't as well supported. Try this:

find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or -type f -print0 | xargs -0 -L1 bash -c 'if test "$(grep --files-with-matches --binary-files=without-match --max-count=1 --regexp='.*' "$0")" && test "$(tail -b 1 "$0")"; then echo "No new line at end of $0."; false; fi'

[per1234]

Sorry, I got that wrong it should be -c, not -b:

find . -path './.git' -prune -or -path './avr/travis-ci' -prune -or -path './avr/cores/MCUdude_corefiles' -prune -or -path './avr/bootloaders' -prune -or -type f -print0 | xargs -0 -L1 bash -c 'if test "$(grep --files-with-matches --binary-files=without-match --max-count=1 --regexp='.*' "$0")" && test "$(tail -c 1 "$0")"; then echo "No new line at end of $0."; false; fi'

[MCUdude]

Thanks, this worked!

[SpenceKonde]

Is there a way to do a similar thing automatically if you normally use the windows github client, rather than raw git on Linux? I'm just not that hardcore, but since I started using travis, at least 50% of my commits have been to fix trailing whitespace, codespell, or missing end of line on files. u_u

[per1234]

The Git hooks I shared above will work with a Git client as well as when using Git from command line.

Please let me know if you have any troubles with the hooks or ideas for how this can be made better. I only started working with Git hooks less than a week ago and the more I use them, the more I'm feeling like the git stash/git stash pop approach I settled on is not the way to go. The alternative of checking out to a temporary folder where the check happens would be pretty slow with a monster repository like the MCUdude cores though.

I use a Git client (Git Extensions) 97% of the time because I find a good GUI to be faster than typing out commands on the command line. I only use the command line for things the GUI doesn't do, or does poorly.

[JAndrassy]

Hans, you could PR the changes for the Ethernet library to the library repo. There are important pull requests now, so I expect a release in the near future. Support for W6100, W5100S are those important PRs

[per1234]

@JAndrassy by "library repo" do you mean the repo for the official Arduino Ethernet library:
https://github.com/arduino-libraries/ethernet

I know MCUdude already submitted one to the PaulStoffregen/Ethernet repository for compatibility with the AT90CAN32/64/128: PaulStoffregen/Ethernet#33

I do think it would be ideal to get that change made in the Arduino Ethernet library because that might pave the way to one day being able to remove the Ethernet library from this repository. That would eliminate the maintenance burden of keeping a bundled copy of the library up to date in this repo and simplify the Travis CI build a bit. I believe that most of the enhancements from PaulStoffregen/Ethernet were incorporated into arduino-libraries/Ethernet for the 2.0.0 release of the library.

[JAndrassy]

@JAndrassy by "library repo" do you mean the repo for the official Arduino Ethernet library:
https://github.com/arduino-libraries/ethernet

yes.

[per1234]

I have published the Git hooks and "fix scripts" I shared above (minus the MCUdude core-specific folder exclusions) to this repository:
https://github.com/per1234/formatting-checks