Changeset View
Standalone View
Mk/Scripts/qa.sh
Show First 20 Lines • Show All 816 Lines • ▼ Show 20 Lines | no_arch() { | ||||
$(list_stagedir_elfs \ | $(list_stagedir_elfs \ | ||||
| file -F $'\1' -f - -N \ | | file -F $'\1' -f - -N \ | ||||
| grep -aE 'ELF .* [LM]SB .*, .*, version [0-9]+ \(FreeBSD\)' \ | | grep -aE 'ELF .* [LM]SB .*, .*, version [0-9]+ \(FreeBSD\)' \ | ||||
| cut -f 1 -d $'\1') | | cut -f 1 -d $'\1') | ||||
EOF | EOF | ||||
return $rc | return $rc | ||||
} | } | ||||
# If an non rubygem-port has a 'Gemfile' file | |||||
# it is checked with bundle to be sure | |||||
# all dependencies are satisfied. | |||||
# Without the check missing/wrong dependencies | |||||
# are just found when executing the application | |||||
gemfiledeps() | |||||
{ | |||||
# skip check if port does not use ruby at all | |||||
if [ -z "$USE_RUBY" ]; then | |||||
lifanov: You also need to gate it on something like USE_RUBY. | |||||
return 0 | |||||
fi | |||||
Done Inline ActionsCan you pick a different variable name please? It's confusing because you are looking for files named literally Gemfile. lifanov: Can you pick a different variable name please? It's confusing because you are looking for files… | |||||
Done Inline ActionsChange to GemfilePath tz: Change to GemfilePath | |||||
# skip check if port is a rubygem-* one; they have no Gemfiles | |||||
Done Inline ActionsYou forgot the case: # No results presents a blank line from heredoc. [ -z "${Gemfile}" ] && continue mat: You forgot the case:
```
# No results presents a blank line from heredoc. | |||||
Done Inline ActionsAdded. tz: Added. | |||||
if [ "${PKGBASE%%-*}" = "rubygem" ]; then | |||||
return 0 | |||||
Done Inline ActionsThis should be [ ! -f mat: This should be `[ ! -f` | |||||
Done Inline ActionsChanged. tz: Changed. | |||||
fi | |||||
# advise install of bundler if its not present for check | |||||
Done Inline ActionsExit if "bundle" is not available or pick something else to use. lifanov: Exit if "bundle" is not available or pick something else to use. | |||||
Done Inline Actionstest that bundle exists? mat: test that bundle exists? | |||||
if ! bundle -v > /dev/null 2>&1; then | |||||
Not Done Inline Actionsif ! type -p bundle; then mat: if ! type -p bundle; then | |||||
Not Done Inline ActionsAntonie reports in the PR:
I'm unfamiliar with this check. Any suggestion how to improve? tz: Antonie reports in the PR:
> The check looks wrong, when bundle is not installed:
>
>
>… | |||||
Done Inline ActionsTry "command -v bundle" instead. lifanov: Try "command -v bundle" instead. | |||||
Not Done Inline ActionsIt works as wanted when the error output is redirected. tz: It works as wanted when the error output is redirected. | |||||
notice "Please install sysutils/rubygem-bundler for additional Gemfile-checks" | |||||
return 0 | |||||
fi | |||||
Done Inline ActionsAdd a "warn" or "err" here. This needs to include a message to the porter about what's wrong. lifanov: Add a "warn" or "err" here. This needs to include a message to the porter about what's wrong. | |||||
Done Inline Actionsbundle creates a very good error message - isn't this enough? tz: bundle creates a very good error message - isn't this enough? | |||||
Done Inline ActionsHmm, I'm not sure. We do want messages to be uniform for stage-qa targets, but if bundle error message is good, it's a judgement call. lifanov: Hmm, I'm not sure. We do want messages to be uniform for stage-qa targets, but if bundle error… | |||||
Done Inline Actionsbundle prints what gem is missing or which version is not satisfied. It also prints out the load order and dependency chain of the gems. This is very helpful but far away from a uniform stage-qa message. tz: bundle prints what gem is missing or which version is not satisfied. It also prints out the… | |||||
Done Inline ActionsIf you think it's good, it might be good. Can you paste a sample output into the "Summary" field above please? lifanov: If you think it's good, it might be good. Can you paste a sample output into the "Summary"… | |||||
Done Inline Actions
It seems some Gemfiles loading other Gemfiles and this will fail bundle. No idea how to handle this at the moment. tz: > cd /usr/ports/www/gitlab && make patch && bundle check --gemfile work/gitlabhq-9.1.9/Gemfile… | |||||
# locate the Gemfile(s) | |||||
Done Inline ActionsNever test $?. Instead, use if: if ! bundle ...; then mat: Never test `$?`. Instead, use if:
```
if ! bundle ...; then
``` | |||||
while read -r f; do | |||||
Done Inline Actionscould you use a lower case variable, possibly shorter, like "f" for file ? mat: could you use a lower case variable, possibly shorter, like "f" for file ? | |||||
Not Done Inline ActionsYes, of course. But i wonder why? I have a hard time to read the other stages because of their cryptic writing and the massive absence of documentation. I think a good named variable makes life much easier. And that a shell script does not have the same restrictions like microcode or other embedded stuff. Or is it just to be consistent to the rest? Or something else? tz: Yes, of course. But i wonder why?
I have a hard time to read the other stages because of their… | |||||
Not Done Inline Actionslowercase because we only use uppercase variables for things that come through the environment like make(1) variables that are passed through. Also we do not use camelcase. As for shorter, I find it makes it harder to read with long variable names, especially as there is only one variable here. mat: lowercase because we only use uppercase variables for things that come through the environment… | |||||
# no results presents a blank line from heredoc | |||||
Done Inline ActionsShouldn't this be in WRKSRC instead ? mat: Shouldn't this be in WRKSRC instead ? | |||||
Done Inline ActionsI took this part from lifanovs patch. I did find STAGEDIR convenient since its a stage-qa. Also a short try says it does not work with WRKSRC. tz: I took this part from lifanovs patch. I did find STAGEDIR convenient since its a stage-qa. Also… | |||||
Done Inline ActionsI think STAGEDIR is correct. Gemfile gets installed. lifanov: I think STAGEDIR is correct. Gemfile gets installed. | |||||
[ -z "$f" ] && continue | |||||
Done Inline ActionsYou need to quote the variable. mat: You need to quote the variable. | |||||
# if there is none everything is fine - stop here | |||||
[ ! -f "$f" ] && return 0; | |||||
Done Inline ActionsQuote variable. mat: Quote variable. | |||||
# use bundle to check if Gemfile is satisfied | |||||
# if bundle returns 1 the Gemfile is not satisfied | |||||
Done Inline ActionsStill not testing that bundle exists before calling it. mat: Still not testing that bundle exists before calling it. | |||||
# and so stage-qa isn't also | |||||
if ! bundle check --dry-run --gemfile $f > /dev/null 2>&1; then | |||||
err "Dependencies defined in ${f} are not satisfied" | |||||
return 1; | |||||
fi | |||||
done <<-EOF | |||||
$(find ${STAGEDIR} -name Gemfile) | |||||
Done Inline Actionsindentation problem here. mat: indentation problem here. | |||||
Not Done Inline ActionsNow fixed :) tz: Now fixed :) | |||||
EOF | |||||
return 0 | |||||
} | |||||
checks="shebang symlinks paths stripped desktopfileutils sharedmimeinfo" | checks="shebang symlinks paths stripped desktopfileutils sharedmimeinfo" | ||||
checks="$checks suidfiles libtool libperl prefixvar baselibs terminfo" | checks="$checks suidfiles libtool libperl prefixvar baselibs terminfo" | ||||
checks="$checks proxydeps sonames perlcore no_arch" | checks="$checks proxydeps sonames perlcore no_arch gemfiledeps" | ||||
ret=0 | ret=0 | ||||
cd ${STAGEDIR} | cd ${STAGEDIR} | ||||
for check in ${checks}; do | for check in ${checks}; do | ||||
${check} || ret=1 | ${check} || ret=1 | ||||
done | done | ||||
exit ${ret} | exit ${ret} |
You also need to gate it on something like USE_RUBY.