* [XEN PATCH] automation/eclair: Make report browsing URL configurable. @ 2025-06-26 6:38 Nicola Vetrini 2025-06-26 10:08 ` Anthony PERARD 0 siblings, 1 reply; 4+ messages in thread From: Nicola Vetrini @ 2025-06-26 6:38 UTC (permalink / raw) To: xen-devel Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder, consulting, Nicola Vetrini, Doug Goldstein, Anthony Perard, Alessandro Zucchelli Currently, the URL where the ECLAIR MISRA C scan reports are saved is hardcoded; making it configurable allows multiple runners and storage servers to be used without resorting to publishing all artifacts to the same report server. Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> --- Note: this is a key enabler for drafting more comprehensive community analyses that can be possibly split between different machines --- .../eclair_analysis/ECLAIR/action.settings | 31 +++++++++++++++---- .../eclair_analysis/ECLAIR/action_push.sh | 2 +- automation/gitlab-ci/analyze.yaml | 2 ++ automation/scripts/eclair | 13 +++++++- 4 files changed, 40 insertions(+), 8 deletions(-) diff --git a/automation/eclair_analysis/ECLAIR/action.settings b/automation/eclair_analysis/ECLAIR/action.settings index 1577368b613b..f822f0ea66d7 100644 --- a/automation/eclair_analysis/ECLAIR/action.settings +++ b/automation/eclair_analysis/ECLAIR/action.settings @@ -14,9 +14,6 @@ autoPRRepository="${AUTO_PR_REPOSITORY:-}" # Customized autoPRBranch="${AUTO_PR_BRANCH:-}" -# Customized -artifactsRoot=/var/local/eclair - case "${ci}" in github) # To be customized @@ -166,12 +163,34 @@ esac ECLAIR_BIN_DIR=/opt/bugseng/eclair/bin/ -artifactsDir="${artifactsRoot}/xen-project.ecdf/${repository}/ECLAIR_${ANALYSIS_KIND}" +# Artifacts URL served by the eclair_report server +if [ -z "${MACHINE_ARTIFACTS_ROOT}" ]; +then + echo "WARNING: No artifacts root supplied, using default" +fi +if [ -z "${MACHINE_ECDF_DIR}" ]; +then + echo "WARNING: No ecdf dir supplied, using default" +fi +artifactsRoot="${MACHINE_ARTIFACTS_ROOT:-/var/local/eclair}" +artifactsEcdfDir="${MACHINE_ECDF_DIR:-xen-project.ecdf}" +artifactsDir="${artifactsRoot}/${artifactsEcdfDir}/${repository}/ECLAIR_${ANALYSIS_KIND}" subDir="${subDir}${variantSubDir}" jobHeadline="${jobHeadline}${variantHeadline}" -# Customized -eclairReportUrlPrefix=https://saas.eclairit.com:3787 +# Remote eclair_report hosting server +if [ -z "${MACHINE_HOST}" ]; +then + echo "WARNING: No machine host supplied, using default" +fi +if [ -z "${MACHINE_PORT}" ]; +then + echo "WARNING: No machine port supplied, using default" +fi + +eclairReportHost="${MACHINE_HOST:-saas.eclairit.com}" +eclairReportPort="${MACHINE_PORT:-3787}" +eclairReportUrlPrefix="https://${eclairReportHost}:${eclairReportPort}" jobDir="${artifactsDir}/${subDir}/${jobId}" updateLog="${analysisOutputDir}/update.log" diff --git a/automation/eclair_analysis/ECLAIR/action_push.sh b/automation/eclair_analysis/ECLAIR/action_push.sh index 45215fbf005b..5002b48522e2 100755 --- a/automation/eclair_analysis/ECLAIR/action_push.sh +++ b/automation/eclair_analysis/ECLAIR/action_push.sh @@ -1,6 +1,6 @@ #!/bin/sh -set -eu +set -eux usage() { echo "Usage: $0 WTOKEN ANALYSIS_OUTPUT_DIR" >&2 diff --git a/automation/gitlab-ci/analyze.yaml b/automation/gitlab-ci/analyze.yaml index 5b00b9f25ca6..f027c6bc90b1 100644 --- a/automation/gitlab-ci/analyze.yaml +++ b/automation/gitlab-ci/analyze.yaml @@ -8,6 +8,8 @@ ENABLE_ECLAIR_BOT: "n" AUTO_PR_BRANCH: "staging" AUTO_PR_REPOSITORY: "xen-project/xen" + MACHINE_ARTIFACTS_ROOT: "/space" + MACHINE_ECDF_DIR: "XEN.ecdf" script: - ./automation/scripts/eclair 2>&1 | tee "${LOGFILE}" artifacts: diff --git a/automation/scripts/eclair b/automation/scripts/eclair index 0a2353c20a92..7020eaa0982f 100755 --- a/automation/scripts/eclair +++ b/automation/scripts/eclair @@ -1,4 +1,15 @@ -#!/bin/sh -eu +#!/bin/sh -eux + +# Runner-specific variables +ex=0 +export "$(env | grep MACHINE_ARTIFACTS_ROOT)" || ex=$? +[ "${ex}" = 0 ] || exit "${ex}" +export "$(env | grep MACHINE_ECDF_DIR)" || ex=$? +[ "${ex}" = 0 ] || exit "${ex}" +export "$(env | grep MACHINE_HOST)" || ex=$? +[ "${ex}" = 0 ] || exit "${ex}" +export "$(env | grep MACHINE_PORT)" || ex=$? +[ "${ex}" = 0 ] || exit "${ex}" ECLAIR_ANALYSIS_DIR=automation/eclair_analysis ECLAIR_DIR="${ECLAIR_ANALYSIS_DIR}/ECLAIR" -- 2.43.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [XEN PATCH] automation/eclair: Make report browsing URL configurable. 2025-06-26 6:38 [XEN PATCH] automation/eclair: Make report browsing URL configurable Nicola Vetrini @ 2025-06-26 10:08 ` Anthony PERARD 2025-06-27 8:39 ` Nicola Vetrini 0 siblings, 1 reply; 4+ messages in thread From: Anthony PERARD @ 2025-06-26 10:08 UTC (permalink / raw) To: Nicola Vetrini Cc: xen-devel, sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder, consulting, Doug Goldstein, Alessandro Zucchelli On Thu, Jun 26, 2025 at 08:38:18AM +0200, Nicola Vetrini wrote: > diff --git a/automation/eclair_analysis/ECLAIR/action.settings b/automation/eclair_analysis/ECLAIR/action.settings > index 1577368b613b..f822f0ea66d7 100644 > --- a/automation/eclair_analysis/ECLAIR/action.settings > +++ b/automation/eclair_analysis/ECLAIR/action.settings > @@ -14,9 +14,6 @@ autoPRRepository="${AUTO_PR_REPOSITORY:-}" > # Customized > autoPRBranch="${AUTO_PR_BRANCH:-}" > > -# Customized > -artifactsRoot=/var/local/eclair > - > case "${ci}" in > github) > # To be customized > @@ -166,12 +163,34 @@ esac > > ECLAIR_BIN_DIR=/opt/bugseng/eclair/bin/ > > -artifactsDir="${artifactsRoot}/xen-project.ecdf/${repository}/ECLAIR_${ANALYSIS_KIND}" > +# Artifacts URL served by the eclair_report server > +if [ -z "${MACHINE_ARTIFACTS_ROOT}" ]; You don't need a ';' if you have `then` on the next line ;-) > +then > + echo "WARNING: No artifacts root supplied, using default" > +fi > +if [ -z "${MACHINE_ECDF_DIR}" ]; > +then > + echo "WARNING: No ecdf dir supplied, using default" > +fi > +artifactsRoot="${MACHINE_ARTIFACTS_ROOT:-/var/local/eclair}" > +artifactsEcdfDir="${MACHINE_ECDF_DIR:-xen-project.ecdf}" Do we need to separate varables for these two? It might be a bit simpler to have: artifactsRoot=/var/local/eclair/xen-project.ecdf unless there's other path than *.ecdf. But in any case, two separate variables looks fine too. > +artifactsDir="${artifactsRoot}/${artifactsEcdfDir}/${repository}/ECLAIR_${ANALYSIS_KIND}" > subDir="${subDir}${variantSubDir}" > jobHeadline="${jobHeadline}${variantHeadline}" > > -# Customized > -eclairReportUrlPrefix=https://saas.eclairit.com:3787 > +# Remote eclair_report hosting server > +if [ -z "${MACHINE_HOST}" ]; > +then > + echo "WARNING: No machine host supplied, using default" > +fi > +if [ -z "${MACHINE_PORT}" ]; > +then > + echo "WARNING: No machine port supplied, using default" > +fi > + > +eclairReportHost="${MACHINE_HOST:-saas.eclairit.com}" > +eclairReportPort="${MACHINE_PORT:-3787}" > +eclairReportUrlPrefix="https://${eclairReportHost}:${eclairReportPort}" Please, don't make the port number mandatory. Can you merge both host and port in the same variable? This part seems to be called "authority": https://en.wikipedia.org/wiki/URL#Syntax Also, don't use `MACHINE` as prefix/namespace for these new variables, in a pipeline context, "machine" could be many things. Maybe "ECLAIR_REPORT_HOST" for this one? With the default been: ECLAIR_REPORT_HOST=saas.eclairit.com:3787 I wonder if "https" should be configurable as well, but I guess there shouldn't be any need for it as we probably don't want to serve reports over http. > > jobDir="${artifactsDir}/${subDir}/${jobId}" > updateLog="${analysisOutputDir}/update.log" > diff --git a/automation/eclair_analysis/ECLAIR/action_push.sh b/automation/eclair_analysis/ECLAIR/action_push.sh > index 45215fbf005b..5002b48522e2 100755 > --- a/automation/eclair_analysis/ECLAIR/action_push.sh > +++ b/automation/eclair_analysis/ECLAIR/action_push.sh > @@ -1,6 +1,6 @@ > #!/bin/sh > > -set -eu > +set -eux Left over change from debugging? > > usage() { > echo "Usage: $0 WTOKEN ANALYSIS_OUTPUT_DIR" >&2 > diff --git a/automation/gitlab-ci/analyze.yaml b/automation/gitlab-ci/analyze.yaml > index 5b00b9f25ca6..f027c6bc90b1 100644 > --- a/automation/gitlab-ci/analyze.yaml > +++ b/automation/gitlab-ci/analyze.yaml > @@ -8,6 +8,8 @@ > ENABLE_ECLAIR_BOT: "n" > AUTO_PR_BRANCH: "staging" > AUTO_PR_REPOSITORY: "xen-project/xen" > + MACHINE_ARTIFACTS_ROOT: "/space" > + MACHINE_ECDF_DIR: "XEN.ecdf" Is this the right place for these variables? Shouldn't they be set on gitlab (at project or repo scope) or even set by the runner itself. > script: > - ./automation/scripts/eclair 2>&1 | tee "${LOGFILE}" > artifacts: > diff --git a/automation/scripts/eclair b/automation/scripts/eclair > index 0a2353c20a92..7020eaa0982f 100755 > --- a/automation/scripts/eclair > +++ b/automation/scripts/eclair > @@ -1,4 +1,15 @@ > -#!/bin/sh -eu > +#!/bin/sh -eux > + > +# Runner-specific variables > +ex=0 > +export "$(env | grep MACHINE_ARTIFACTS_ROOT)" || ex=$? > +[ "${ex}" = 0 ] || exit "${ex}" That's a really complicated way to check a variable is set... Exporting a variable that's already in env isn't useful, and I think `ex` is only ever set to `0`. It seems that `dash` just exit if you do `export=""`. You could simply do: : ${MACHINE_ARTIFACTS_ROOT:?Missing MACHINE_ARTIFACTS_ROOT variable} : ${MACHINE_ECDF_DIR:?Missing MACHINE_ECDF_DIR variable} To check that the variables are set. Or nothing, if you add `set -u` to the script (instead of the one -u in the sheband which might be ignored if one run `sh ./eclair` instead of just `./eclair`.) Also the variable should come from the env, as nothing sets it, so no need to for that. ( The `:` at the begining of the line is necessary, and behave the same way as `true` does. We need it because ${parm:?msg} is expanded.) Or you could use `if [ -z "${param}" ]` if ${param:?msg} is too obscure. We would just have "parameter not set" instead of a nicer message, due to `set -u`. Thanks, -- Anthony PERARD ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [XEN PATCH] automation/eclair: Make report browsing URL configurable. 2025-06-26 10:08 ` Anthony PERARD @ 2025-06-27 8:39 ` Nicola Vetrini 2025-06-27 15:18 ` Anthony PERARD 0 siblings, 1 reply; 4+ messages in thread From: Nicola Vetrini @ 2025-06-27 8:39 UTC (permalink / raw) To: Anthony PERARD Cc: xen-devel, sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder, consulting, Doug Goldstein, Alessandro Zucchelli On 2025-06-26 12:08, Anthony PERARD wrote: > On Thu, Jun 26, 2025 at 08:38:18AM +0200, Nicola Vetrini wrote: >> diff --git a/automation/eclair_analysis/ECLAIR/action.settings >> b/automation/eclair_analysis/ECLAIR/action.settings >> index 1577368b613b..f822f0ea66d7 100644 >> --- a/automation/eclair_analysis/ECLAIR/action.settings >> +++ b/automation/eclair_analysis/ECLAIR/action.settings >> @@ -14,9 +14,6 @@ autoPRRepository="${AUTO_PR_REPOSITORY:-}" >> # Customized >> autoPRBranch="${AUTO_PR_BRANCH:-}" >> >> -# Customized >> -artifactsRoot=/var/local/eclair >> - >> case "${ci}" in >> github) >> # To be customized >> @@ -166,12 +163,34 @@ esac >> >> ECLAIR_BIN_DIR=/opt/bugseng/eclair/bin/ >> >> -artifactsDir="${artifactsRoot}/xen-project.ecdf/${repository}/ECLAIR_${ANALYSIS_KIND}" >> +# Artifacts URL served by the eclair_report server >> +if [ -z "${MACHINE_ARTIFACTS_ROOT}" ]; > > You don't need a ';' if you have `then` on the next line ;-) > Hi Anthony, yeah, missed that. Thanks >> +then >> + echo "WARNING: No artifacts root supplied, using default" >> +fi >> +if [ -z "${MACHINE_ECDF_DIR}" ]; >> +then >> + echo "WARNING: No ecdf dir supplied, using default" >> +fi >> +artifactsRoot="${MACHINE_ARTIFACTS_ROOT:-/var/local/eclair}" >> +artifactsEcdfDir="${MACHINE_ECDF_DIR:-xen-project.ecdf}" > > Do we need to separate varables for these two? It might be a bit > simpler > to have: > artifactsRoot=/var/local/eclair/xen-project.ecdf > unless there's other path than *.ecdf. But in any case, two separate > variables looks fine too. > The main reason why I used two variables is that one may have differently-named .ecdf directories on different machines, but there is no strong reason have two variables >> +artifactsDir="${artifactsRoot}/${artifactsEcdfDir}/${repository}/ECLAIR_${ANALYSIS_KIND}" >> subDir="${subDir}${variantSubDir}" >> jobHeadline="${jobHeadline}${variantHeadline}" >> >> -# Customized >> -eclairReportUrlPrefix=https://saas.eclairit.com:3787 >> +# Remote eclair_report hosting server >> +if [ -z "${MACHINE_HOST}" ]; >> +then >> + echo "WARNING: No machine host supplied, using default" >> +fi >> +if [ -z "${MACHINE_PORT}" ]; >> +then >> + echo "WARNING: No machine port supplied, using default" >> +fi >> + >> +eclairReportHost="${MACHINE_HOST:-saas.eclairit.com}" >> +eclairReportPort="${MACHINE_PORT:-3787}" >> +eclairReportUrlPrefix="https://${eclairReportHost}:${eclairReportPort}" > > Please, don't make the port number mandatory. Can you merge both host > and port in the same variable? This part seems to be called > "authority": > > https://en.wikipedia.org/wiki/URL#Syntax > > Also, don't use `MACHINE` as prefix/namespace for these new variables, > in a pipeline context, "machine" could be many things. Maybe > "ECLAIR_REPORT_HOST" for this one? With the default been: > > ECLAIR_REPORT_HOST=saas.eclairit.com:3787 > I can merge host and port and change the variable prefix, but I think there is a misunderstanding. This address is used both as the base for report browsing and pushing the results. While we should alter the latter (e.g., ECLAIR_REPORT_PROXY_HOST) to point to the proxy so that the proxy is shown in the CI logs, the address where the results are pushed is fixed and set in the docker runner env. This is not ideal, but I didn't find a better way with GitLab CI to let the analysis push locally. > I wonder if "https" should be configurable as well, but I guess there > shouldn't be any need for it as we probably don't want to serve reports > over http. > Yeah, I don't think so >> >> jobDir="${artifactsDir}/${subDir}/${jobId}" >> updateLog="${analysisOutputDir}/update.log" >> diff --git a/automation/eclair_analysis/ECLAIR/action_push.sh >> b/automation/eclair_analysis/ECLAIR/action_push.sh >> index 45215fbf005b..5002b48522e2 100755 >> --- a/automation/eclair_analysis/ECLAIR/action_push.sh >> +++ b/automation/eclair_analysis/ECLAIR/action_push.sh >> @@ -1,6 +1,6 @@ >> #!/bin/sh >> >> -set -eu >> +set -eux > > Left over change from debugging? > Yes >> >> usage() { >> echo "Usage: $0 WTOKEN ANALYSIS_OUTPUT_DIR" >&2 >> diff --git a/automation/gitlab-ci/analyze.yaml >> b/automation/gitlab-ci/analyze.yaml >> index 5b00b9f25ca6..f027c6bc90b1 100644 >> --- a/automation/gitlab-ci/analyze.yaml >> +++ b/automation/gitlab-ci/analyze.yaml >> @@ -8,6 +8,8 @@ >> ENABLE_ECLAIR_BOT: "n" >> AUTO_PR_BRANCH: "staging" >> AUTO_PR_REPOSITORY: "xen-project/xen" >> + MACHINE_ARTIFACTS_ROOT: "/space" >> + MACHINE_ECDF_DIR: "XEN.ecdf" > > Is this the right place for these variables? Shouldn't they be set on > gitlab (at project or repo scope) or even set by the runner itself. > Well, it was easier to set them there for debugging. The idea was to potentially override them at the runner config level, if needed. >> script: >> - ./automation/scripts/eclair 2>&1 | tee "${LOGFILE}" >> artifacts: >> diff --git a/automation/scripts/eclair b/automation/scripts/eclair >> index 0a2353c20a92..7020eaa0982f 100755 >> --- a/automation/scripts/eclair >> +++ b/automation/scripts/eclair >> @@ -1,4 +1,15 @@ >> -#!/bin/sh -eu >> +#!/bin/sh -eux >> + >> +# Runner-specific variables >> +ex=0 >> +export "$(env | grep MACHINE_ARTIFACTS_ROOT)" || ex=$? >> +[ "${ex}" = 0 ] || exit "${ex}" > > That's a really complicated way to check a variable is set... > Exporting a variable that's already in env isn't useful, and I think > `ex` is only ever set to `0`. It seems that `dash` just exit if you do > `export=""`. > > You could simply do: > > : ${MACHINE_ARTIFACTS_ROOT:?Missing MACHINE_ARTIFACTS_ROOT > variable} > : ${MACHINE_ECDF_DIR:?Missing MACHINE_ECDF_DIR variable} > > To check that the variables are set. Or nothing, if you add `set -u` to > the script (instead of the one -u in the sheband which might be ignored > if one run `sh ./eclair` instead of just `./eclair`.) Also the variable > should come from the env, as nothing sets it, so no need to for that. > > ( The `:` at the begining of the line is necessary, and behave the same > way as `true` does. We need it because ${parm:?msg} is expanded.) > > Or you could use `if [ -z "${param}" ]` if ${param:?msg} is too > obscure. > We would just have "parameter not set" instead of a nicer message, due > to `set -u`. > I agree it is ugly and counterintuitive, but the core idea here is that the variable is set but not exported for some reason, so just `export VAR` does not behave in the same way as the incantation `export "$(env | grep MACHINE_ARTIFACTS_ROOT)"` iirc. I'll double check if there's a better way to achieve this (other than switching to bash in the shebang). Thanks, -- Nicola Vetrini, B.Sc. Software Engineer BUGSENG (https://bugseng.com) LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [XEN PATCH] automation/eclair: Make report browsing URL configurable. 2025-06-27 8:39 ` Nicola Vetrini @ 2025-06-27 15:18 ` Anthony PERARD 0 siblings, 0 replies; 4+ messages in thread From: Anthony PERARD @ 2025-06-27 15:18 UTC (permalink / raw) To: Nicola Vetrini Cc: xen-devel, sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder, consulting, Doug Goldstein, Alessandro Zucchelli On Fri, Jun 27, 2025 at 10:39:21AM +0200, Nicola Vetrini wrote: > On 2025-06-26 12:08, Anthony PERARD wrote: > > On Thu, Jun 26, 2025 at 08:38:18AM +0200, Nicola Vetrini wrote: > > > +eclairReportHost="${MACHINE_HOST:-saas.eclairit.com}" > > > +eclairReportPort="${MACHINE_PORT:-3787}" > > > +eclairReportUrlPrefix="https://${eclairReportHost}:${eclairReportPort}" > > > > Please, don't make the port number mandatory. Can you merge both host > > and port in the same variable? This part seems to be called "authority": > > > > https://en.wikipedia.org/wiki/URL#Syntax > > > > Also, don't use `MACHINE` as prefix/namespace for these new variables, > > in a pipeline context, "machine" could be many things. Maybe > > "ECLAIR_REPORT_HOST" for this one? With the default been: > > > > ECLAIR_REPORT_HOST=saas.eclairit.com:3787 > > > > I can merge host and port and change the variable prefix, but I think there > is a misunderstanding. This address is used both as the base for report > browsing and pushing the results. Well, the patch is all about "report browsing URL" as stated in the subject. > While we should alter the latter (e.g., > ECLAIR_REPORT_PROXY_HOST) to point to the proxy so that the proxy is shown "proxy"? That's implementation detail, there's no need to know that the host used to browse the result is different than the host where the result are stored. (I mean having "proxy" in the name of the variable holding detail of how to access the reports, been a part of an url or just the host) > in the CI logs, the address where the results are pushed is fixed and set in > the docker runner env. This is not ideal, but I didn't find a better way > with GitLab CI to let the analysis push locally. Having the configuration for where to push the result in the runner env seems like the best option to me, if that report server is tied to the eclair runner. > > > diff --git a/automation/scripts/eclair b/automation/scripts/eclair > > > index 0a2353c20a92..7020eaa0982f 100755 > > > --- a/automation/scripts/eclair > > > +++ b/automation/scripts/eclair > > > @@ -1,4 +1,15 @@ > > > -#!/bin/sh -eu > > > +#!/bin/sh -eux > > > + > > > +# Runner-specific variables > > > +ex=0 > > > +export "$(env | grep MACHINE_ARTIFACTS_ROOT)" || ex=$? > > > +[ "${ex}" = 0 ] || exit "${ex}" > > > > That's a really complicated way to check a variable is set... > > Exporting a variable that's already in env isn't useful, and I think > > `ex` is only ever set to `0`. It seems that `dash` just exit if you do > > `export=""`. > > > > You could simply do: > > > > : ${MACHINE_ARTIFACTS_ROOT:?Missing MACHINE_ARTIFACTS_ROOT variable} > > : ${MACHINE_ECDF_DIR:?Missing MACHINE_ECDF_DIR variable} > > > > To check that the variables are set. Or nothing, if you add `set -u` to > > the script (instead of the one -u in the sheband which might be ignored > > if one run `sh ./eclair` instead of just `./eclair`.) Also the variable > > should come from the env, as nothing sets it, so no need to for that. > > > > ( The `:` at the begining of the line is necessary, and behave the same > > way as `true` does. We need it because ${parm:?msg} is expanded.) > > > > Or you could use `if [ -z "${param}" ]` if ${param:?msg} is too obscure. > > We would just have "parameter not set" instead of a nicer message, due > > to `set -u`. > > > > I agree it is ugly and counterintuitive, but the core idea here is that the > variable is set but not exported for some reason, so just `export VAR` does > not behave in the same way as the incantation `export "$(env | grep > MACHINE_ARTIFACTS_ROOT)"` iirc. I'll double check if there's a better way to > achieve this (other than switching to bash in the shebang). Isn't that script `./eclair` the beginning of the script? I can't find anything that `source` this file so it must be. Which mean, if MACHINE_ARTIFACTS_ROOT is set, it is exported, because nothing sets it between the beginning of the script and this line. So there's no need to check that the variable is exported (if it is not exported, it is a bug in the shell). The only thing left to do is to check that it is set and non-zero length. But I still wonder why you do that here, since "action.settings" also check the variable and use a default value. There's no need to use bash for this. (They are plenty of reason to use bash instead of posix shell, but that is just not one of them.) Cheers, -- Anthony PERARD ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-06-27 15:18 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-06-26 6:38 [XEN PATCH] automation/eclair: Make report browsing URL configurable Nicola Vetrini 2025-06-26 10:08 ` Anthony PERARD 2025-06-27 8:39 ` Nicola Vetrini 2025-06-27 15:18 ` Anthony PERARD
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.