Re: Mingw task for Cirrus CI

Justin Pryzby <pryzby(at)telsasoft(dot)com>, 19 Ağu 2022 Cum, 05:34 tarihinde şunu
yazdı:

> Inline notes about changes since the last version.
>
> On Thu, Jul 28, 2022 at 05:44:28PM -0500, Justin Pryzby wrote:
> > I think the "only_if" should allow separately running one but not both
> of the
> > windows instances, like:
> >
> > + only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' ||
> $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*mingw64'
> >
> > I'm not sure, but maybe this task should only run "by request", and omit
> the
> > first condition:
> >
> > + only_if: $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*mingw64'
>
> The patch shouldn't say this during development, or else cfbot doesn't run
> it..
> Oops.
>
Actually, making MinGW task optional for now might make sense. Due to
windows resource limits on Cirrus CI and slow builds on Windows, adding
this task as non-optional may not be an efficient decision
I think that continuing with this patch by changing MinGW to optional for
now, instead of waiting for more resource on Cirrus or faster builds on
Windows, could be better. I don't see any harm.

+ only_if: $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-include:[^\n]*mingw.* ||
> $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*mingw.*'

Added this line to allow run only mingw task or run all tasks including
mingw.

What do you all think about this change? Does it make sense?

Thanks for your contributions/reviews Justin!

> I think it should include something like
> >
> > + setup_additional_packages_script: |
> > + REM C:\msys64\usr\bin\pacman.exe -S --noconfirm ...
> >
> > Let's see what others think about those.
> >
> > Do you know if this handles logging of crash dumps ?
>
> It does now, although I hardcoded "postgres.exe" ...
>

merged this with my patch

> + setup_additional_packages_script: |
> > + REM C:\msys64\usr\bin\pacman.exe -S --noconfirm busybox
>
> This should include choco, too.

Added pacman.exe line. Do we really need choco here? I don't think mingw
would require any package via choco.
Also is ending pacman.exe line with busybox intentional? I just added that
line with "..." at the end instead of any package name.

>
>
> - CXXFLAGS='-Og -ggdb'"
> > + CXXFLAGS='-Og -ggdb' && break;
> > + rm -v ${CCACHE_DIR}/configure.cache;
> > + done
>
> I noticed that this doesn't seem to do the right thing with the exit
> status -
> configure can fail without cirrusci noticing, and then the build fails at
> the
> next step.

merged.

>
>
> for item in `find "$sourcetree" -name Makefile -print -o -name
> GNUmakefile -print | grep -v "$sourcetree/doc/src/sgml/images/"`; do
> > - filename=`expr "$item" : "$sourcetree\(.*\)"`
> > - if test ! -f "${item}.in"; then
> > - if cmp "$item" "$buildtree/$filename" >/dev/null 2>&1; then : ;
> else
> > - ln -fs "$item" "$buildtree/$filename" || exit 1
> > - fi
> > - fi
> > + filename=${item#$sourcetree}
> > + [ -e "$buildtree/$filename" ] && continue
>
> I fixed this to check for ".in" files as intended.
>
> It'd be a lot better if the image didn't take so long to start. :(
>

One question would be that should this patch include "prep_buildtree"? It
doesn't seem to me like it's directly related to adding MinGW into CI but
more like an improvement for builds on Windows.
Maybe we can make it a seperate patch if it's necessary.

What do you think?

TAR: "c:/windows/system32/tar.exe"
>

Sharing a new version of the patch. It also moves the above line so that it
will apply to mingw task too. Otherwise mingw task was failing.

Thanks,
Melih