Bug 227109 - Mk/Scripts, cleanup using devel/hs-ShellCheck
Summary: Mk/Scripts, cleanup using devel/hs-ShellCheck
Status: New
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Ports Framework (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Port Management Team
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-03-30 11:56 UTC by Mathieu Arnold
Modified: 2019-01-11 15:14 UTC (History)
1 user (show)

See Also:
mat: exp-run?


Attachments
based on r465949 (105.83 KB, patch)
2018-03-30 11:56 UTC, Mathieu Arnold
no flags Details | Diff
base commit: r470764 (106.16 KB, patch)
2018-05-24 12:00 UTC, Mathieu Arnold
no flags Details | Diff
base commit: r471274 (3.94 KB, patch)
2018-06-01 16:24 UTC, Mathieu Arnold
no flags Details | Diff
base commit: r471994 (80.03 KB, patch)
2018-06-08 09:31 UTC, Mathieu Arnold
no flags Details | Diff
base commit: r489959 (37.69 KB, patch)
2019-01-11 15:14 UTC, Mathieu Arnold
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mathieu Arnold freebsd_committer 2018-03-30 11:56:44 UTC
Created attachment 191974 [details]
based on r465949
Comment 1 Mathieu Arnold freebsd_committer 2018-03-30 12:35:07 UTC
There is review D13970 too.
Comment 2 Antoine Brodin freebsd_committer 2018-04-02 07:29:01 UTC
When starting the bulk,  I noticed this in poudriere output:

eval: -:: not found
eval: COPYRIGHT: not found
eval: Stop.: not found
eval: make:: not found
Comment 3 Antoine Brodin freebsd_committer 2018-04-02 07:30:34 UTC
make patch seems broken,  

patch: **** can't open file : No such file or directory
Comment 4 Mathieu Arnold freebsd_committer 2018-05-24 12:00:02 UTC
Created attachment 193646 [details]
base commit: r470764

fixed regressions.
Comment 5 Antoine Brodin freebsd_committer 2018-05-27 17:05:49 UTC
New depends failures:

http://package23.nyi.freebsd.org/data/111i386-default-PR227109/2018-05-27_15h26m18s/logs/errors/pstoedit-3.70_8.log
http://package23.nyi.freebsd.org/data/111i386-default-PR227109/2018-05-27_15h26m18s/logs/errors/nip2-8.6.0_1.log

New deinstall failures ( .metadir/+(|PRE|POST)(INSTALL|DEINSTALL) seem to have the same content twice ):

http://package23.nyi.freebsd.org/data/111i386-default-PR227109/2018-05-27_15h26m18s/logs/errors/condor-8.4.12_1.log
http://package23.nyi.freebsd.org/data/111i386-default-PR227109/2018-05-27_15h26m18s/logs/errors/ja-font-kochi-20030809_5.log
http://package23.nyi.freebsd.org/data/111i386-default-PR227109/2018-05-27_15h26m18s/logs/errors/ja-font-koruri-20161105.log
http://package23.nyi.freebsd.org/data/111i386-default-PR227109/2018-05-27_15h26m18s/logs/errors/ja-font-marumoji-1.0_10.log
http://package23.nyi.freebsd.org/data/111i386-default-PR227109/2018-05-27_15h26m18s/logs/errors/ja-font-mplus-outline-0.6.0.log
http://package23.nyi.freebsd.org/data/111i386-default-PR227109/2018-05-27_15h26m18s/logs/errors/ja-font-sazanami-20040629_6.log
http://package23.nyi.freebsd.org/data/111i386-default-PR227109/2018-05-27_15h26m18s/logs/errors/ja-font-ume-0.0.660.log
http://package23.nyi.freebsd.org/data/111i386-default-PR227109/2018-05-27_15h26m18s/logs/errors/ja-font-vlgothic-20141206_4.log
http://package23.nyi.freebsd.org/data/111i386-default-PR227109/2018-05-27_15h26m18s/logs/errors/p5-jmx4perl-1.11_3.log
http://package23.nyi.freebsd.org/data/111i386-default-PR227109/2018-05-27_15h26m18s/logs/errors/smlnj-110.77.log
Comment 6 Mathieu Arnold freebsd_committer 2018-06-01 15:33:46 UTC
Ok, this is never going to work this way, too many changes, and even with git bisect, it is a pain to fix. I am going to do this incrementally, one kind of change at a time.
Comment 7 commit-hook freebsd_committer 2018-06-01 16:20:47 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  1 16:20:23 UTC 2018
New revision: 471264
URL: https://svnweb.freebsd.org/changeset/ports/471264

Log:
  SC2145: Argument mixes string and array. Use * or separate argument.

  The behavior when concatenating a string and array is rarely intended.
  The preceeding string is prefixed to the first array element, while the
  succeeding string is appended to the last one. The middle array elements
  are unaffected.

  For example, with the parameters foo,bar,baz, "--flag=$@" is equivalent
  to the three arguments "--flag=foo" "bar" "baz".

  PR:		227109
  Submitted by:	mat
  Sponsored by:	Absolight

Changes:
  head/Mk/Scripts/functions.sh
  head/Mk/Scripts/generate-symbols.sh
  head/Mk/Scripts/qa.sh
Comment 8 commit-hook freebsd_committer 2018-06-01 16:20:49 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  1 16:20:27 UTC 2018
New revision: 471265
URL: https://svnweb.freebsd.org/changeset/ports/471265

Log:
  SC2068: Double quote array expansions to avoid re-splitting elements.

  Double quotes around $@ prevents globbing and word splitting of
  individual elements, while still expanding to multiple separate
  arguments.

  Add exceptions when splitting is the intended behavior.

  PR:		227109
  Submitted by:	mat
  Sponsored by:	Absolight

Changes:
  head/Mk/Scripts/depends-list.sh
Comment 9 commit-hook freebsd_committer 2018-06-01 16:20:51 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  1 16:20:30 UTC 2018
New revision: 471266
URL: https://svnweb.freebsd.org/changeset/ports/471266

Log:
  SC2221 & SC2222: This pattern always overrides a later/previous one.

  You have specified multiple patterns in a case statement, where one will
  always override the other. The pattern being overridden is indicated
  with a SC2222 warning.

  PR:		227109
  Submitted by:	mat
  Sponsored by:	Absolight

Changes:
  head/Mk/Scripts/check_leftovers.sh
Comment 10 commit-hook freebsd_committer 2018-06-01 16:20:53 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  1 16:20:33 UTC 2018
New revision: 471267
URL: https://svnweb.freebsd.org/changeset/ports/471267

Log:
  SC2198: Arrays don't work as operands in [ ]. Use a loop (or concatenate with * instead of @).

  Array expansions become a series of words in [ .. ]. Operators expect
  single words only.

  PR:		227109
  Submitted by:	mat
  Sponsored by:	Absolight

Changes:
  head/Mk/Scripts/checksum.sh
Comment 11 commit-hook freebsd_committer 2018-06-01 16:21:56 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  1 16:20:36 UTC 2018
New revision: 471268
URL: https://svnweb.freebsd.org/changeset/ports/471268

Log:
  SC2034: <some var> appears unused. Verify it or export it.

  Variables not used for anything are often associated with bugs, so
  ShellCheck warns about them.

  PR:		227109
  Submitted by:	mat
  Sponsored by:	Absolight

Changes:
  head/Mk/Scripts/functions.sh
Comment 12 commit-hook freebsd_committer 2018-06-01 16:21:58 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  1 16:20:39 UTC 2018
New revision: 471269
URL: https://svnweb.freebsd.org/changeset/ports/471269

Log:
  SC2163: Exporting an expansion rather than a variable.

  export takes a variable name, but shellcheck has noticed that you give
  it an expanded variable instead. The problematic code does not export
  MYVAR but a variable called foo if any.

  Add exception when using indirections where the variable to extract is
  actually the value of the variable.

  PR:		227109
  Submitted by:	mat
  Sponsored by:	Absolight

Changes:
  head/Mk/Scripts/functions.sh
Comment 13 commit-hook freebsd_committer 2018-06-01 16:22:01 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  1 16:20:42 UTC 2018
New revision: 471270
URL: https://svnweb.freebsd.org/changeset/ports/471270

Log:
  SC2091: Remove surrounding $() to avoid executing output.

  ShellCheck has detected that you have a command that just consists of a
  command substitution.

  This is typically done in order to try to get the shell to execute a
  command, because $(..) does indeed execute commands. However, it's also
  replaced by the output of that command.

  PR:		227109
  Submitted by:	mat
  Sponsored by:	Absolight

Changes:
  head/Mk/Scripts/qa.sh
Comment 14 commit-hook freebsd_committer 2018-06-01 16:22:03 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  1 16:20:45 UTC 2018
New revision: 471271
URL: https://svnweb.freebsd.org/changeset/ports/471271

Log:
  SC2153: Possible misspelling: PORTNAME may not be assigned, but portname is.

  ShellCheck has noticed that you reference a variable that is not
  assigned in the script, which has a name remarkably similar to one that
  is explicitly assigned. You should verify that the variable name is
  spelled correctly.

  PR:		227109
  Submitted by:	mat
  Sponsored by:	Absolight

Changes:
  head/Mk/Scripts/qa.sh
Comment 15 commit-hook freebsd_committer 2018-06-01 16:22:05 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  1 16:20:48 UTC 2018
New revision: 471272
URL: https://svnweb.freebsd.org/changeset/ports/471272

Log:
  SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

  cd can fail for a variety of reasons: misspelled paths, missing
  directories, missing permissions, broken symlinks and more.

  If/when it does, the script will keep going and do all its operations in
  the wrong directory. This can be messy, especially if the operations
  involve creating or deleting a lot of files.

  PR:		227109
  Submitted by:	mat
  Sponsored by:	Absolight

Changes:
  head/Mk/Scripts/qa.sh
Comment 16 commit-hook freebsd_committer 2018-06-01 16:22:07 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  1 16:20:51 UTC 2018
New revision: 471273
URL: https://svnweb.freebsd.org/changeset/ports/471273

Log:
  SC2155: Declare and assign separately to avoid masking return values.

  In the original code, the return value of mycmd is ignored, and export
  will instead always return true. This may prevent conditionals, set -e
  and traps from working correctly.

  PR:		227109
  Submitted by:	mat
  Sponsored by:	Absolight

Changes:
  head/Mk/Scripts/smart_makepatch.sh
Comment 17 commit-hook freebsd_committer 2018-06-01 16:22:09 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  1 16:20:55 UTC 2018
New revision: 471274
URL: https://svnweb.freebsd.org/changeset/ports/471274

Log:
  SC2006: Use $(..) instead of legacy `..`.

  Backtick command substitution `STATEMENT` is legacy syntax with several
  issues.

   - It has a series of undefined behaviors related to quoting in POSIX.
   - It imposes a custom escaping mode with surprising results.
   - It's exceptionally hard to nest.

  $(STATEMENT) command substitution has none of these problems, and is
  therefore strongly encouraged.

  PR:		227109
  Submitted by:	mat
  Sponsored by:	Absolight

Changes:
  head/Mk/Scripts/actual-package-depends.sh
  head/Mk/Scripts/create-manifest.sh
  head/Mk/Scripts/find-lib.sh
  head/Mk/Scripts/qa.sh
Comment 18 Mathieu Arnold freebsd_committer 2018-06-01 16:24:10 UTC
Created attachment 193895 [details]
base commit: r471274

First non "noop" commit:

    SC2046: Quote this to prevent word splitting.

    When command expansions are unquoted, word splitting and globbing will
    occur. This often manifests itself by breaking when filenames contain
    spaces.

    Trying to fix it by adding quotes or escapes to the data will not work.
    Instead, quote the command substitution itself.

    If the command substitution outputs multiple pieces of data, use a loop
    instead.

    Add an exception when using set -- where splitting is intended.
Comment 19 Antoine Brodin freebsd_committer 2018-06-07 09:01:15 UTC
Exp-run looks fine for this patch
Comment 20 commit-hook freebsd_committer 2018-06-08 09:27:25 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  8 09:26:21 UTC 2018
New revision: 471988
URL: https://svnweb.freebsd.org/changeset/ports/471988

Log:
  SC2046: Quote this to prevent word splitting.

  When command expansions are unquoted, word splitting and globbing will
  occur. This often manifests itself by breaking when filenames contain
  spaces.

  Trying to fix it by adding quotes or escapes to the data will not work.
  Instead, quote the command substitution itself.

  If the command substitution outputs multiple pieces of data, use a loop
  instead.

  Add an exception when using set -- where splitting is intended.

  PR:             227109
  Submitted by:   mat
  Exp-run by:	antoine
  Sponsored by:   Absolight

Changes:
  head/Mk/Scripts/check_leftovers.sh
  head/Mk/Scripts/depends-list.sh
  head/Mk/Scripts/find-lib.sh
  head/Mk/Scripts/functions.sh
  head/Mk/Scripts/smart_makepatch.sh
Comment 21 commit-hook freebsd_committer 2018-06-08 09:27:27 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  8 09:26:25 UTC 2018
New revision: 471989
URL: https://svnweb.freebsd.org/changeset/ports/471989

Log:
  SC2185: Some finds don't have a default path. Specify '.' explicitly.

  (false positive, split flags to avoid triggering it.)

  PR:             227109
  Submitted by:   mat
  Sponsored by:   Absolight

Changes:
  head/Mk/Scripts/check-stagedir.sh
Comment 22 commit-hook freebsd_committer 2018-06-08 09:27:29 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  8 09:26:28 UTC 2018
New revision: 471990
URL: https://svnweb.freebsd.org/changeset/ports/471990

Log:
  Don't exec with a pipe afterwards, it is weird.

  PR:             227109
  Submitted by:   mat
  Sponsored by:   Absolight

Changes:
  head/Mk/Scripts/plist_sub_sed_sort.sh
Comment 23 commit-hook freebsd_committer 2018-06-08 09:27:31 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  8 09:26:32 UTC 2018
New revision: 471991
URL: https://svnweb.freebsd.org/changeset/ports/471991

Log:
  SC2015: Note that A && B || C is not if-then-else. C may run when A is true.

  It's common to use A && B to run B when A is true, and A || C to run C
  when A is false.

  However, combining them into A && B || C is not the same as if A then B
  else C.

  In this case, if A is true but B is false, C will run.

  If an if clause is used instead, this problem is avoided.

  PR:             227109
  Submitted by:   mat
  Sponsored by:   Absolight

Changes:
  head/Mk/Scripts/qa.sh
Comment 24 commit-hook freebsd_committer 2018-06-08 09:27:33 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  8 09:26:35 UTC 2018
New revision: 471992
URL: https://svnweb.freebsd.org/changeset/ports/471992

Log:
  SC2162: read without -r will mangle backslashes.

  By default, read will interpret backslashes before spaces and line
  feeds, and otherwise strip them. This is rarely expected or desired.

  Normally you just want to read data, which is what read -r does. You
  should always use -r unless you have a good reason not to.

  PR:             227109
  Submitted by:   mat
  Sponsored by:   Absolight

Changes:
  head/Mk/Scripts/check-stagedir.sh
  head/Mk/Scripts/check_leftovers.sh
  head/Mk/Scripts/generate-symbols.sh
  head/Mk/Scripts/qa.sh
Comment 25 commit-hook freebsd_committer 2018-06-08 09:27:36 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  8 09:26:39 UTC 2018
New revision: 471993
URL: https://svnweb.freebsd.org/changeset/ports/471993

Log:
  SC2035: Use ./*glob* or -- *glob* so names with dashes won't become options.

  Since files and arguments are strings passed the same way, programs
  can't properly determine which is which, and rely on dashes to determine
  what's what.

  A file named -f (touch -- -f) will not be deleted by the problematic
  code. It will instead be interpreted as a command line option, and rm
  will even report success.

  Using ./* will instead cause the glob to be expanded into ./-f, which no
  program will treat as an option.

  It is not possible to use `-f *` because -f only forces the next
  argument to be a directory, a later directory named -delete would mess
  things up.

  PR:             227109
  Submitted by:   mat
  Sponsored by:   Absolight

Changes:
  head/Mk/Scripts/smart_makepatch.sh
Comment 26 commit-hook freebsd_committer 2018-06-08 09:27:38 UTC
A commit references this bug:

Author: mat
Date: Fri Jun  8 09:26:42 UTC 2018
New revision: 471994
URL: https://svnweb.freebsd.org/changeset/ports/471994

Log:
  SC1004: This backslash+linefeed is literal. Break outside single quotes if you just want to break the line.

  You have a single quoted string containing a backslash followed by a
  linefeed (newline). Unlike double quotes or unquoted strings, this has
  no special meaning. The string will contain a literal backslash and a
  linefeed.

  If you wanted to break the line but not add a linefeed to the string,
  stop the single quote, break the line, and reopen it.

  PR:             227109
  Submitted by:   mat
  Sponsored by:   Absolight

Changes:
  head/Mk/Scripts/qa.sh
  head/Mk/Scripts/smart_makepatch.sh
Comment 27 Mathieu Arnold freebsd_committer 2018-06-08 09:31:10 UTC
Created attachment 194078 [details]
base commit: r471994

Ok, next patch, this one will break stuff.
Comment 28 Antoine Brodin freebsd_committer 2018-06-11 05:52:42 UTC
Mmmm it seems all the failures mentionned in comment #5 are present
Comment 29 Mathieu Arnold freebsd_committer 2019-01-11 15:14:52 UTC
Created attachment 201029 [details]
base commit: r489959

Ok, remove the quoting of everything and try with the rest of the patches.

I was able to test build a few ports, and to start bulk -a with this.