Summary: | Mk/Scripts, cleanup using devel/hs-ShellCheck | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | Ports & Packages | Reporter: | Mathieu Arnold <mat> | ||||||||||||||||
Component: | Ports Framework | Assignee: | Port Management Team <portmgr> | ||||||||||||||||
Status: | Closed FIXED | ||||||||||||||||||
Severity: | Affects Only Me | CC: | ports-bugs | ||||||||||||||||
Priority: | --- | Flags: | mat:
exp-run?
|
||||||||||||||||
Version: | Latest | ||||||||||||||||||
Hardware: | Any | ||||||||||||||||||
OS: | Any | ||||||||||||||||||
Attachments: |
|
There is review D13970 too. 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 make patch seems broken, patch: **** can't open file : No such file or directory Created attachment 193646 [details]
base commit: r470764
fixed regressions.
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 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. 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 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 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 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 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 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 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 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 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 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 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 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.
Exp-run looks fine for this patch 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 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 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 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 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 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 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 Created attachment 194078 [details]
base commit: r471994
Ok, next patch, this one will break stuff.
Mmmm it seems all the failures mentionned in comment #5 are present 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.
Created attachment 201756 [details]
base commit: r492166
rebase patch.
Created attachment 201757 [details]
base commit: r492198
add one more commit.
this is probably a end less work and we should can close this PR |
Created attachment 191974 [details] based on r465949