Changeset View
Standalone View
Mk/Scripts/qa.sh
Show First 20 Lines • Show All 824 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 | ||||
} | } | ||||
gemdeps() | gemdeps() | ||||
lifanov: You also need to gate it on something like USE_RUBY. | |||||
{ | { | ||||
rc=0 | rc=0 | ||||
if [ "${PKGBASE%%-*}" = "rubygem" ]; then | if [ "${PKGBASE%%-*}" = "rubygem" ]; then | ||||
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 | |||||
while read -r l; do | while read -r l; do | ||||
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 [ -n "${l}" ]; then | if [ -n "${l}" ]; then | ||||
name=${l%% *} | name=${l%% *} | ||||
Done Inline ActionsThis should be [ ! -f mat: This should be `[ ! -f` | |||||
Done Inline ActionsChanged. tz: Changed. | |||||
vers=${l#* } | vers=${l#* } | ||||
while read -r v; do | while read -r v; do | ||||
if ! while read -r p; do | if ! while read -r p; do | ||||
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? | |||||
${LOCALBASE}/bin/ruby -e "puts 'OK' if Gem::Dependency.new('${name}','${v}').match?('${name}','${p}')" | ${LOCALBASE}/bin/ruby -e "puts 'OK' if Gem::Dependency.new('${name}','${v}').match?('${name}','${p}')" | ||||
done | grep -qFx OK; then | done | grep -qFx OK; then | ||||
err RubyGem dependency ${name} ${v} is not satisfied. | err RubyGem dependency ${name} ${v} is not satisfied. | ||||
rc=1 | rc=1 | ||||
fi <<-EOF | fi <<-EOF | ||||
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… | |||||
$(${LOCALBASE}/bin/gem list -e "${name}" \ | $(${LOCALBASE}/bin/gem list -e "${name}" \ | ||||
Done Inline ActionsNever test $?. Instead, use if: if ! bundle ...; then mat: Never test `$?`. Instead, use if:
```
if ! bundle ...; then
``` | |||||
| sed "s|.*(\(.*\))|\1|" \ | | sed "s|.*(\(.*\))|\1|" \ | ||||
| tr -d ' ' \ | | tr -d ' ' \ | ||||
| tr , '\n') | | tr , '\n') | ||||
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. | |||||
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. | |||||
EOF | EOF | ||||
done <<-EOF | done <<-EOF | ||||
$(while echo "${vers}" | grep -q '"'; do | $(while echo "${vers}" | grep -q '"'; do | ||||
echo "${vers}" | cut -d '"' -f2 | echo "${vers}" | cut -d '"' -f2 | ||||
vers=$(echo "${vers}"|cut -d '"' -f3-) | vers=$(echo "${vers}"|cut -d '"' -f3-) | ||||
done) | done) | ||||
EOF | EOF | ||||
fi | fi | ||||
done <<-EOF | done <<-EOF | ||||
$(grep -a 'add_runtime_dependency' ${STAGEDIR}${PREFIX}/lib/ruby/gems/*/specifications/${PORTNAME}-*.gemspec \ | $(grep -a 'add_runtime_dependency' ${STAGEDIR}${PREFIX}/lib/ruby/gems/*/specifications/${PORTNAME}-*.gemspec \ | ||||
| sed 's|.*<\(.*\)>.*\[\(.*\)\])|\1 \2|' \ | | sed 's|.*<\(.*\)>.*\[\(.*\)\])|\1 \2|' \ | ||||
| sort -u) | | sort -u) | ||||
EOF | EOF | ||||
fi | fi | ||||
return $rc | return $rc | ||||
} | } | ||||
# If an non rubygem-port has a 'Gemfile' file | |||||
# it is checked with bundle to be sure | |||||
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… | |||||
# all dependencies are satisfied. | |||||
# Without the check missing/wrong dependencies | |||||
# are just found when executing the application | |||||
Done Inline ActionsYou need to quote the variable. mat: You need to quote the variable. | |||||
gemfiledeps() | |||||
{ | |||||
# skip check if port does not use ruby at all | |||||
Done Inline ActionsQuote variable. mat: Quote variable. | |||||
if [ -z "$USE_RUBY" ]; then | |||||
return 0 | |||||
fi | |||||
Done Inline ActionsStill not testing that bundle exists before calling it. mat: Still not testing that bundle exists before calling it. | |||||
# skip check if port is a rubygem-* one; they have no Gemfiles | |||||
if [ "${PKGBASE%%-*}" = "rubygem" ]; then | |||||
return 0 | |||||
fi | |||||
# advise install of bundler if its not present for check | |||||
if ! type -p bundle; then | |||||
Done Inline Actionsindentation problem here. mat: indentation problem here. | |||||
Not Done Inline ActionsNow fixed :) tz: Now fixed :) | |||||
notice "Please install sysutils/rubygem-bundler for additional Gemfile-checks" | |||||
return 0 | |||||
fi | |||||
# locate the Gemfile(s) | |||||
while read -r f; do | |||||
# no results presents a blank line from heredoc | |||||
[ -z "$f" ] && continue | |||||
# if there is none everything is fine - stop here | |||||
[ ! -f "$f" ] && return 0; | |||||
# use bundle to check if Gemfile is satisfied | |||||
# if bundle returns 1 the Gemfile is not satisfied | |||||
# and so stage-qa isn't also | |||||
if ! bundle check --dry-run --gemfile $f > /dev/null 2>&1; then | |||||
warn "Dependencies defined in ${f} are not satisfied" | |||||
fi | |||||
done <<-EOF | |||||
$(find ${STAGEDIR} -name Gemfile) | |||||
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 gemdeps" | checks="$checks proxydeps sonames perlcore no_arch gemdeps 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.