| Message ID | PAVP189MB245804C450EAB74D69070756F5CC2@PAVP189MB2458.EURP189.PROD.OUTLOOK.COM |
|---|---|
| State | Accepted |
| Delegated to: | Alin Gabriel Serdean |
| Headers | show |
| Series | [ovs-dev] windows: Fixed MSYS detection in CCCL. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/cirrus-robot | success | cirrus build: passed |
On Fri, Feb 28, 2025 at 3:32 PM Frank Wagner <frank.wagner@dbosoft.eu> wrote: > > Fixed a path mapping problem (slash replaced by path) with current msys versions. > > cccl assumes that MACHTYPE contains "-msys" on msys and then configures slashes to '-' to avoid path mapping problems with slashes. However, at least in the current MSYS version, MACHTYPE reports as cygwin. A better way to detect MSYS is to use the MSYSTEM variable. > I'm assuming that this check has been failing for a while, but not causing cccl to fail, but in current MSYS versions the path mapping logic has changed. > > See also https://github.com/swig/cccl/issues/20 > > Signed-off-by: Frank Wagner <frank.wagner@dbosoft.eu> > > --- > build-aux/cccl | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/build-aux/cccl b/build-aux/cccl > index e2426fb3e..855d24c6c 100644 > --- a/build-aux/cccl > +++ b/build-aux/cccl > @@ -33,14 +33,22 @@ EOF > exit $1 > } > > -case $MACHTYPE in > - *-msys) Hello Frank, Why not just add another case for "*-cygwin)" ? Cheers, M > - slash="-" > - ;; > - *) > - slash="/" > - ;; > -esac > + > + > +# Check for MSYS which now reports itself as cygwin in MACHTYPE > +if [[ -n "$MSYSTEM" ]]; then > + slash="-" > +else > + # fallback to old behavior > + case $MACHTYPE in > + *-msys) > + slash="-" > + ;; > + *) > + slash="/" > + ;; > + esac > +fi > # prog specifies the program that should be run (cl.exe or link.exe) > # We'll assume cl to start out > prog=cl > -- > 2.48.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Because cygwin should use / but MSYS should use - instead of a slash. But I had a discussion with the cccl maintainer on https://github.com/swig/cccl/issues/20 - he has noticed same issue and raised in in MSYS. They recommend using MSYSTEM to detect MSYS, but it is currently open if the fallback to the old behavior is necessary.
Hi Frank, Thank you for the patch. I do have a question regarding the build environment, I'm assuming from the discussion you are using cygwin + msys which is not part of our documentation. My question is why would this approach adds on top of the documented msys2 environment which we also use in our CI ( https://github.com/openvswitch/ovs/blob/main/appveyor.yml#L61) -- Alin On Tue, Mar 4, 2025 at 10:20 PM Frank Wagner <frank.wagner@dbosoft.eu> wrote: > Because cygwin should use / but MSYS should use - instead of a slash. > > But I had a discussion with the cccl maintainer on > https://github.com/swig/cccl/issues/20 - he has noticed same issue and > raised in in MSYS. They recommend using MSYSTEM to detect MSYS, but it is > currently open if the fallback to the old behavior is necessary. > > ________________________________ > Von: Mike Pattrick <mkp@redhat.com> > Gesendet: Dienstag, 4. März 2025 18:27 > An: Frank Wagner <frank.wagner@dbosoft.eu> > Cc: dev@openvswitch.org <dev@openvswitch.org> > Betreff: Re: [ovs-dev] [PATCH] windows: Fixed MSYS detection in CCCL. > > On Fri, Feb 28, 2025 at 3:32 PM Frank Wagner <frank.wagner@dbosoft.eu> > wrote: > > > > Fixed a path mapping problem (slash replaced by path) with current msys > versions. > > > > cccl assumes that MACHTYPE contains "-msys" on msys and then configures > slashes to '-' to avoid path mapping problems with slashes. However, at > least in the current MSYS version, MACHTYPE reports as cygwin. A better way > to detect MSYS is to use the MSYSTEM variable. > > I'm assuming that this check has been failing for a while, but not > causing cccl to fail, but in current MSYS versions the path mapping logic > has changed. > > > > See also https://github.com/swig/cccl/issues/20 > > > > Signed-off-by: Frank Wagner <frank.wagner@dbosoft.eu> > > > > --- > > build-aux/cccl | 24 ++++++++++++++++-------- > > 1 file changed, 16 insertions(+), 8 deletions(-) > > > > diff --git a/build-aux/cccl b/build-aux/cccl > > index e2426fb3e..855d24c6c 100644 > > --- a/build-aux/cccl > > +++ b/build-aux/cccl > > @@ -33,14 +33,22 @@ EOF > > exit $1 > > } > > > > -case $MACHTYPE in > > - *-msys) > > Hello Frank, > > Why not just add another case for "*-cygwin)" ? > > Cheers, > M > > > - slash="-" > > - ;; > > - *) > > - slash="/" > > - ;; > > -esac > > + > > + > > +# Check for MSYS which now reports itself as cygwin in MACHTYPE > > +if [[ -n "$MSYSTEM" ]]; then > > + slash="-" > > +else > > + # fallback to old behavior > > + case $MACHTYPE in > > + *-msys) > > + slash="-" > > + ;; > > + *) > > + slash="/" > > + ;; > > + esac > > +fi > > # prog specifies the program that should be run (cl.exe or link.exe) > > # We'll assume cl to start out > > prog=cl > > -- > > 2.48.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Hi Alina, No, using MSYS with ucrt, same as appveyor. See the discussion on GitHub: the MSYS team has changed its identifier to cygwin in Februar Release. Gesendet von Outlook für Android<https://aka.ms/AAb9ysg>
Sorry Alin, auto correction on my mobile... Here the link to the change in MSYS: https://www.msys2.org/news/#2025-02-14-moving-msys2-closer-to-cygwin
On Fri, Feb 28, 2025 at 3:32 PM Frank Wagner <frank.wagner@dbosoft.eu> wrote: > > Fixed a path mapping problem (slash replaced by path) with current msys versions. > > cccl assumes that MACHTYPE contains "-msys" on msys and then configures slashes to '-' to avoid path mapping problems with slashes. However, at least in the current MSYS version, MACHTYPE reports as cygwin. A better way to detect MSYS is to use the MSYSTEM variable. > I'm assuming that this check has been failing for a while, but not causing cccl to fail, but in current MSYS versions the path mapping logic has changed. > > See also https://github.com/swig/cccl/issues/20 > > Signed-off-by: Frank Wagner <frank.wagner@dbosoft.eu> Seems reasonable! Acked-by: Mike Pattrick <mkp@redhat.com>
I had no idea msys2 is going the cygwin route, thanks for clarifying this. Will take a look over all the patches later today / this weekend. Alin. On Thu, Mar 6, 2025 at 7:11 PM Mike Pattrick <mkp@redhat.com> wrote: > On Fri, Feb 28, 2025 at 3:32 PM Frank Wagner <frank.wagner@dbosoft.eu> > wrote: > > > > Fixed a path mapping problem (slash replaced by path) with current msys > versions. > > > > cccl assumes that MACHTYPE contains "-msys" on msys and then configures > slashes to '-' to avoid path mapping problems with slashes. However, at > least in the current MSYS version, MACHTYPE reports as cygwin. A better way > to detect MSYS is to use the MSYSTEM variable. > > I'm assuming that this check has been failing for a while, but not > causing cccl to fail, but in current MSYS versions the path mapping logic > has changed. > > > > See also https://github.com/swig/cccl/issues/20 > > > > Signed-off-by: Frank Wagner <frank.wagner@dbosoft.eu> > > Seems reasonable! > > Acked-by: Mike Pattrick <mkp@redhat.com> > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Tested and applied on master. Thank you Frank and Mike On Fri, Mar 7, 2025 at 3:01 PM Alin Serdean <alinserdean@gmail.com> wrote: > I had no idea msys2 is going the cygwin route, thanks for clarifying this. > > Will take a look over all the patches later today / this weekend. > > Alin. > > On Thu, Mar 6, 2025 at 7:11 PM Mike Pattrick <mkp@redhat.com> wrote: > >> On Fri, Feb 28, 2025 at 3:32 PM Frank Wagner <frank.wagner@dbosoft.eu> >> wrote: >> > >> > Fixed a path mapping problem (slash replaced by path) with current msys >> versions. >> > >> > cccl assumes that MACHTYPE contains "-msys" on msys and then configures >> slashes to '-' to avoid path mapping problems with slashes. However, at >> least in the current MSYS version, MACHTYPE reports as cygwin. A better way >> to detect MSYS is to use the MSYSTEM variable. >> > I'm assuming that this check has been failing for a while, but not >> causing cccl to fail, but in current MSYS versions the path mapping logic >> has changed. >> > >> > See also https://github.com/swig/cccl/issues/20 >> > >> > Signed-off-by: Frank Wagner <frank.wagner@dbosoft.eu> >> >> Seems reasonable! >> >> Acked-by: Mike Pattrick <mkp@redhat.com> >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >
diff --git a/build-aux/cccl b/build-aux/cccl index e2426fb3e..855d24c6c 100644 --- a/build-aux/cccl +++ b/build-aux/cccl @@ -33,14 +33,22 @@ EOF exit $1 } -case $MACHTYPE in - *-msys) - slash="-" - ;; - *) - slash="/" - ;; -esac + + +# Check for MSYS which now reports itself as cygwin in MACHTYPE +if [[ -n "$MSYSTEM" ]]; then + slash="-" +else + # fallback to old behavior + case $MACHTYPE in + *-msys) + slash="-" + ;; + *) + slash="/" + ;; + esac +fi # prog specifies the program that should be run (cl.exe or link.exe) # We'll assume cl to start out prog=cl
Fixed a path mapping problem (slash replaced by path) with current msys versions. cccl assumes that MACHTYPE contains "-msys" on msys and then configures slashes to '-' to avoid path mapping problems with slashes. However, at least in the current MSYS version, MACHTYPE reports as cygwin. A better way to detect MSYS is to use the MSYSTEM variable. I'm assuming that this check has been failing for a while, but not causing cccl to fail, but in current MSYS versions the path mapping logic has changed. See also https://github.com/swig/cccl/issues/20 Signed-off-by: Frank Wagner <frank.wagner@dbosoft.eu> --- build-aux/cccl | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-)