Opened 6 years ago

Closed 5 years ago

#15316 closed defect (fixed)

Make gf2x respect SAGE_FAT_BINARY and use --libdir

Reported by: jpflori Owned by:
Priority: critical Milestone: sage-6.4
Component: packages: standard Keywords: spkg gf2x
Cc: Merged in:
Authors: Jean-Pierre Flori, Erik Massop Reviewers: Jeroen Demeyer, Jean-Pierre Flori
Report Upstream: N/A Work issues:
Branch: 969de52 (Commits) Commit: 969de52313ee407d874e9f7b318111c144de534e
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

When SAGE_FAT_BINARY is set we should pick a generic value for hwdir. We should also use --libdir so that gf2x works ok on OpenSuse? and similar systems.

Upstream: https://gforge.inria.fr/tracker/index.php?func=detail&aid=16566&group_id=1874&atid=6979

Change History (68)

comment:1 Changed 5 years ago by jpflori

  • Authors set to Jean-Pierre Flori
  • Branch set to u/jpflori/ticket/15316
  • Commit set to a278bc91cc19a43f6f8bcfe5c08db486e804f8cc
  • Keywords spkg gf2x added
  • Status changed from new to needs_review

New commits:

a278bc9Let gf2x use --libdir and make SAGE_FAT_BINARY work.

comment:2 Changed 5 years ago by jdemeyer

I'm not entirely sure how portable ln -sf is. I recommend changing it to a rm -f + ln -s instead.

comment:3 Changed 5 years ago by git

  • Commit changed from a278bc91cc19a43f6f8bcfe5c08db486e804f8cc to 2371b3586b7b656293f036dc6e0d467946d02ad8

Branch pushed to git repo; I updated commit sha1. New commits:

2371b35Replace symlink in a more portable way.

comment:4 Changed 5 years ago by jpflori

Done.

comment:5 Changed 5 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to needs_work

Given that all x86_64 processors support SSE2, why should we disable that? We should disable it for i386, but not for x86_64.

Another small comment: I don't like

echo "Disabling optimizations."

Why not make it

echo "Building a generic gf2x library."

comment:6 Changed 5 years ago by jpflori

For SSE2, I said the same thing to gf2x devs and they told me sse2 was not part of the x86_64. See https://gforge.inria.fr/tracker/index.php?func=detail&aid=16566&group_id=1874&atid=6979

The other point is I was too lazy to check if the system was actually 32 or 64 bits in spkg-install.

comment:7 follow-up: Changed 5 years ago by jdemeyer

Perhaps SSE2 is not part of the official standard, but at least all x86_64 CPUs support it, so it would say it is "de facto" standard. Also GCC will generate SSE+SSE2 code by default on x86_64, which is a very strong precedent.

Is this not just a matter of not replacing the symlink?

comment:8 follow-up: Changed 5 years ago by jdemeyer

The gf2x devs are wrong: http://www.x86-64.org/documentation/abi.pdf states on p.125 that sse2 is a required processor feature.

comment:9 in reply to: ↑ 7 Changed 5 years ago by jpflori

Replying to jdemeyer:

Perhaps SSE2 is not part of the official standard, but at least all x86_64 CPUs support it, so it would say it is "de facto" standard. Also GCC will generate SSE+SSE2 code by default on x86_64, which is a very strong precedent.

I agree, and that's what I defended upstream, but Emmanuel didn't hear it that way...

Is this not just a matter of not replacing the symlink?

In fact, if SSE2 is enabled, then we don't even need to replace the symlinks.

Apart from the feedback from upstream which does not want to provide a build system with an option to disable optimizations which would build an SSE2 lib but without further optimizations on x86_64, but just one which would simply point to the generic32/64 folders (so no assembly at all whatever the platform) when enabled.

The other issue is that I did not feel the need to add a bunch of code to spkg-install tyo detect if the system targeted is 32/64 bits, x86 or sparc or whatever, and then pass the correct value to the "hwdir" var (let's say x86, x86_64 and so on depending on the system, which is what I asked for upstream but got as a response: "we'll pass generic32/64 and nothing more depending on the bit length"), when passing unconditionnaly "--disable-sse2 --disable-pclmul" would more or less do the trick (except for the fact it broke the build on x86_64...). Indeed this would really fit in the upstream autotools project, not within our spkg-install script. But you nearly convinced me to submit that upstream....

comment:10 in reply to: ↑ 8 ; follow-up: Changed 5 years ago by jpflori

Replying to jdemeyer:

The gf2x devs are wrong: http://www.x86-64.org/documentation/abi.pdf states on p.125 that sse2 is a required processor feature.

Nice. I'll push that upstream, but how "official" is this document? In fact when upstream told me SSE2 was not required I was not sure what and where the official ABI could be...

comment:11 in reply to: ↑ 10 Changed 5 years ago by jpflori

Replying to jpflori:

Replying to jdemeyer:

The gf2x devs are wrong: http://www.x86-64.org/documentation/abi.pdf states on p.125 that sse2 is a required processor feature.

Nice. I'll push that upstream, but how "official" is this document? In fact when upstream told me SSE2 was not required I was not sure what and where the official ABI could be...

Hum, the about page:

makes it look more official than what the site design made me orignalmly think.

comment:12 follow-up: Changed 5 years ago by jdemeyer

I think you should discuss the ABI thing with upstream and see what they say.

comment:13 in reply to: ↑ 12 Changed 5 years ago by jpflori

Replying to jdemeyer:

I think you should discuss the ABI thing with upstream and see what they say.

Sure, I'll bump the discussion there, pointing to the exact page you spotted and hopefully their position will shift. I'll also directly provide a patch to implement what I (and apprently you) would like to see in.

In between I'll provide these changes here.

comment:14 Changed 5 years ago by git

  • Commit changed from 2371b3586b7b656293f036dc6e0d467946d02ad8 to 0086d4a6c8ca1e1fa3da40a6bdcee21758c2331c

Branch pushed to git repo; I updated commit sha1. New commits:

0086d4aMake generic build of gf2x smarter.

comment:15 Changed 5 years ago by jpflori

  • Description modified (diff)
  • Status changed from needs_work to needs_review

I've tried to tweak the install script to pick up a correct generic value for hwdir depending on cpu/os.

comment:16 Changed 5 years ago by jpflori

  • Status changed from needs_review to needs_work

Hum, that's not enough to prevent the use of specific optimizations flags passed to gcc...

comment:17 Changed 5 years ago by git

  • Commit changed from 0086d4a6c8ca1e1fa3da40a6bdcee21758c2331c to 5159831b64a5a5a3f9c83480f61f031224cd6f5f

Branch pushed to git repo; I updated commit sha1. New commits:

59925a5More fixes for gf2x generic build.
5159831Support SSE2 on x86_64 for gf2x.

comment:18 follow-up: Changed 5 years ago by jdemeyer

Why this?

All other platforms currently supported should build 32 bit by default

I would simply leave non-x86 platform alone.

Last edited 5 years ago by jdemeyer (previous) (diff)

comment:19 follow-up: Changed 5 years ago by jdemeyer

Instead of money-patching configure, you should patch configure.ac and re-run autoconf and add the resulting patch.

comment:20 Changed 5 years ago by jpflori

Replying to jdemeyer:

Why this?

All other platforms currently supported should build 32 bit by default

Because it seemed to me it was the case. I thought of:

  • Solaris/Linux? on sparc* where the default is 32 bit (as per what gcc does) unless SAGE64 is passed (by the way the current filtering might be wrong on Solaris on top of x86_64...).
  • ARM (only 32 bit at the moment, but not even really officially supported anyway).
  • Cygwin should only be x86/x86_64 (does Windows even run on something else?) so is taken care of.
  • Darwin on PPC[!64]/PowerPC (and even PPC64 in fact) which is only supported 32 bit by Sage.
  • non-Darwin ppc64 (let's say POWER? system) are 64 bits.
  • don"t really know on what FreeBSD can run on, but it's not officially supported, and x86/x86_64.

Hum, I forgot (linux) itanium which happens to be 64 bit (only I'd say?)...

All that to say, that my original point was to avoid to develop an overly complicated test script which shouldn't really belong within the install script imho.

comment:21 in reply to: ↑ 18 Changed 5 years ago by jpflori

Replying to jdemeyer:

I would simply leave non-x86 platform alone.

Indeed, this could work as gf2x is smart enough to detect what unsigned long length the compiler use and pick a correct generic* directory.

comment:22 in reply to: ↑ 19 ; follow-ups: Changed 5 years ago by jpflori

Replying to jdemeyer:

Instead of money-patching configure, you should patch configure.ac and re-run autoconf and add the resulting patch.

I'm fed up with overkill patches produced by autoreconfing and don't have access to a machine where your nice autotools spkg is installed (and even with it, autoreconfing can still be a hell sometimes).

comment:23 Changed 5 years ago by git

  • Commit changed from 5159831b64a5a5a3f9c83480f61f031224cd6f5f to ddee57e91708e8599f7c5d8d94aeffe5f0ae2cc5

Branch pushed to git repo; I updated commit sha1. New commits:

ddee57eSimplify generic build of gf2x.

comment:24 in reply to: ↑ 22 ; follow-up: Changed 5 years ago by jpflori

Replying to jpflori:

Replying to jdemeyer:

Instead of money-patching configure, you should patch configure.ac and re-run autoconf and add the resulting patch.

I'm fed up with overkill patches produced by autoreconfing and don't have access to a machine where your nice autotools spkg is installed (and even with it, autoreconfing can still be a hell sometimes).

Also note that is quite exactly what autoreconfing would produce (with all the bullshit removed) or so I hope. I can provide the configure.ac corrsepondig part, which is basically upstream except for the SSE2 on x86_64 part.

comment:25 in reply to: ↑ 24 Changed 5 years ago by jdemeyer

Replying to jpflori:

I can provide the configure.ac corrsepondig part

Please do.

comment:26 in reply to: ↑ 22 ; follow-up: Changed 5 years ago by jdemeyer

Replying to jpflori:

and even with it, autoreconfing can still be a hell sometimes.

That's usually because upstream uses a distro-specific version of autotools. For vanilla versions of autotools, the output that you get with the autotools spkg should be identical to the one in the package.

comment:27 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:28 in reply to: ↑ 26 Changed 5 years ago by jpflori

Replying to jdemeyer:

Replying to jpflori:

and even with it, autoreconfing can still be a hell sometimes.

That's usually because upstream uses a distro-specific version of autotools. For vanilla versions of autotools, the output that you get with the autotools spkg should be identical to the one in the package.

On a side note, would you consider updateing the autotools spkg and gitify it?

I'm currently crafting a proper patch, and while at it I'll aslo include the fixes for SSE2 checks.

comment:29 Changed 5 years ago by git

  • Commit changed from ddee57e91708e8599f7c5d8d94aeffe5f0ae2cc5 to 75888b131d6dbf68506a0096bec7c8b3cd28db34

Branch pushed to git repo; I updated commit sha1. New commits:

fdd7694Properly autoreconfed patches.
75888b1Document new gf2x patch.

comment:30 follow-up: Changed 5 years ago by jpflori

Now it seems that as acinclude.m4 is patched (before configure), autotools want to rerun some magic stuff. Should we manually touch Makefiles?

comment:31 in reply to: ↑ 30 ; follow-up: Changed 5 years ago by jdemeyer

Replying to jpflori:

Should we manually touch Makefiles?

Sure, why not?

comment:32 in reply to: ↑ 31 Changed 5 years ago by jpflori

Replying to jdemeyer:

Replying to jpflori:

Should we manually touch Makefiles?

Sure, why not?

Because:

  • it's more work,
  • it's a pain to maintain as it will be additional code within spkg-install (unless I introduce empty chunks touching these Makefiles in the same patch file touching acinclude.m4 but it feels hackish),
  • I'm lazy :)

comment:33 Changed 5 years ago by git

  • Commit changed from 75888b131d6dbf68506a0096bec7c8b3cd28db34 to 1fb1afe23d755de863d6c8f17b317f2349b8955b

Branch pushed to git repo; I updated commit sha1. New commits:

1fb1afePrevent autotools to regenerate files.

comment:34 Changed 5 years ago by jpflori

  • Status changed from needs_work to needs_review

comment:35 Changed 5 years ago by jdemeyer

I'm confused because it seems that you're solving the same problem twice:

  1. You supply the hwdir option to configure
  2. You supply the --disable-hardware-specific-code option to configure

Wouldn't either of these two work by itself?

A more serious issue is that you should not assume that SSE2 code can be compiled properly, the compiler and assembler need to support it. Hardware support and software support are orthogonal. On x86_64, you may assume hardware support but you still need CHECK_SSE2_SUPPORT() to check for software support.

comment:36 Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_work

comment:37 Changed 5 years ago by jdemeyer

Hang on, working on a patch...

comment:38 Changed 5 years ago by jpflori

Replying to jdemeyer:

I'm confused because it seems that you're solving the same problem twice:

  1. You supply the hwdir option to configure
  2. You supply the --disable-hardware-specific-code option to configure

Wouldn't either of these two work by itself?

Not right now. With "--disable-hardware-specific-code" you only use the generic32, generic64 targets. With only hwdir set, the configure script still checks whether SSE2 and pclmul are supported and can add the corresponding flags to CFLAGS.

A more serious issue is that you should not assume that SSE2 code can be compiled properly, the compiler and assembler need to support it. Hardware support and software support are orthogonal. On x86_64, you may assume hardware support but you still need CHECK_SSE2_SUPPORT() to check for software support.

I agree with the hardware/software dichotomy. The problem is that the CHECK_SSE2_SUPPORT macro does not only checks for SSE2 support but can add things to CFLAGS (for example on 32 bits where -msse2 is needed)... so we cannot just call it unconditionally and that's why it is not called upstream when "--disable-hardware-specific-code" is passed, and the upstream choice to only use generic32/64 seems the right one.

If we want to use the x86_64 hwdir (with sse2) for sage generic builds, we should still pass "--disable-hardware-specific-code" to ensure pclmul is not used, nor sse2 on x86, but still call CHECK_SSE2_SUPPORT() on x86_64 and if that succeeds, set hwdir to x86_64.

The other solution is to modify the symlinks as before so that the x86_64 does not require sse2 (but could still use plain assembly).

comment:39 follow-up: Changed 5 years ago by jdemeyer

Did upstream accept your --disable-hardware-specific-code patch?

comment:40 Changed 5 years ago by jdemeyer

  • Authors changed from Jean-Pierre Flori to Jean-Pierre Flori, Jeroen Demeyer
  • Status changed from needs_work to needs_review

Simplified version of the patches, seems to work (at first sight).

comment:41 in reply to: ↑ 39 Changed 5 years ago by jpflori

  • Authors changed from Jean-Pierre Flori, Jeroen Demeyer to Jean-Pierre Flori

Replying to jdemeyer:

Did upstream accept your --disable-hardware-specific-code patch?

Hummm, this patch is upstream (see commit numbers in SPKG.txt, 162 ands 167 IIRC). Only the x86_64 part which assumes SSE2 support is mine.

comment:42 Changed 5 years ago by jdemeyer

  • Branch changed from u/jpflori/ticket/15316 to u/jdemeyer/ticket/15316
  • Created changed from 10/22/13 11:35:17 to 10/22/13 11:35:17
  • Modified changed from 01/14/14 15:58:49 to 01/14/14 15:58:49

comment:43 Changed 5 years ago by jdemeyer

  • Commit changed from 1fb1afe23d755de863d6c8f17b317f2349b8955b to c059db7ebf1de9c819d549158189d52b4949af8b

I didn't realize the patch was upstream, so perhaps I shouldn't have changed it. Anyway, I'm going to leave this for today...


New commits:

c059db7Simplify gf2x patches

comment:44 Changed 5 years ago by jpflori

  • Branch changed from u/jdemeyer/ticket/15316 to u/jpflori/ticket/15316
  • Commit changed from c059db7ebf1de9c819d549158189d52b4949af8b to 1fb1afe23d755de863d6c8f17b317f2349b8955b

In fact I'm happier with your patch than with what upstream provided, so you could even transfer it upstream.

And note that your patch touch configure.ac after configure.

comment:45 Changed 5 years ago by jdemeyer

About the patch order: you're touching configure anyway...

comment:46 follow-up: Changed 5 years ago by jpflori

  • Branch changed from u/jpflori/ticket/15316 to u/jdemeyer/ticket/15316
  • Commit changed from 1fb1afe23d755de863d6c8f17b317f2349b8955b to c059db7ebf1de9c819d549158189d52b4949af8b

Oops, I mistakenly changed the branch name.

About the patch order: I'd say your patch is the last one so configure.ac will have a more recent timestamp than configure which is what we want to avoid, isn't it? (ok in real life I don't know what granularity timestamps have, so if it is one second, timestamps should be the same).


New commits:

c059db7Simplify gf2x patches

comment:47 Changed 5 years ago by jdemeyer

For the autotools package: see #15123.

comment:48 Changed 5 years ago by jpflori

Your branch fails on ppc64. In fact mine does as well in a similar way but only when SAGE_FAT_BINARY=yes.. Apparently the tuning process tries to build sse2 code.

And this won't work if the compiler does not support SSE2:

++ CHECK_SSE2_SUPPORT()
++ if test x$enable_hardware_specific_code = xyes; then
++ CHECK_PCLMUL_SUPPORT()
++ fi
++ if test x$gf2x_cv_cc_supports_pclmul != xyes ; then
+ hwdir=x86_64

because the x86_64 dir cannot be used without sse2 support (and that's why you could and still can not build on x86_64 with --disable-sse2).

comment:49 Changed 5 years ago by jpflori

The matter is surely that

AM_CONDITIONAL([GF2X_SSE2_AVAILABLE],[test "x$gf2x_cv_cc_supports_sse2" != xno])
AM_CONDITIONAL([GF2X_PCLMUL_AVAILABLE],[test "x$gf2x_cv_cc_supports_pclmul" != xno])

behaves wrongly because the _supports_ vars are left undefined.

comment:50 follow-up: Changed 5 years ago by jdemeyer

Yes, the != xno should be replaced by = xyes I guess.

comment:51 Changed 5 years ago by jpflori

A better and more flexible solution could be to decorrelate the sse2/pclmul tests and the definitions of HAVE_*_SUPPORT. Just as

AM_CONDITIONAL([GF2X_PCLMUL_AVAILABLE],[test "x$gf2x_cv_cc_supports_pclmul" != xno])

which is in configure.ac, the following which is in acinclude.m4 within the SSE2 support macro could be in configure.ac.

 if test "$gf2x_cv_cc_supports_sse2" != "no" ;then
  AC_DEFINE([HAVE_SSE2_SUPPORT],[1],[Define if sse-2 code as present in the source tree is supported by the compiler])
 fi

comment:52 in reply to: ↑ 50 ; follow-up: Changed 5 years ago by jpflori

Replying to jdemeyer:

Yes, the != xno should be replaced by = xyes I guess.

That would also do the trick.

comment:53 in reply to: ↑ 46 Changed 5 years ago by jpflori

Replying to jpflori:

About the patch order: I'd say your patch is the last one so configure.ac will have a more recent timestamp than configure which is what we want to avoid, isn't it? (ok in real life I don't know what granularity timestamps have, so if it is one second, timestamps should be the same).

Could you clarify this as I'm apparently mistaken of why configure.ac should usually be patched before configure?

comment:54 Changed 5 years ago by jpflori

  • Branch changed from u/jdemeyer/ticket/15316 to u/jpflori/ticket/15316
  • Commit changed from c059db7ebf1de9c819d549158189d52b4949af8b to a23168950a9901003604b85a765379d24ac63fb3

I've implemented the changes suggested above.


New commits:

a231689Make the SSE2 stuff work if the compiler does not support SSE2.

comment:55 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:56 Changed 5 years ago by jpflori

Jeroen, any chance you could review this one?

comment:57 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:58 in reply to: ↑ 52 Changed 5 years ago by emassop

  • Status changed from needs_review to needs_work

Replying to jpflori:

Replying to jdemeyer:

Yes, the != xno should be replaced by = xyes I guess.

That would also do the trick.

Not quite: $gf2x_cv_cc_supports_sse2 can be "requires -msse2", and $gf2x_cv_cc_supports_pclmul can be "requires -mpclmul".

Seems more sane to me to run CHECK_SSE2_SUPPORT and CHECK_PCLMUL_SUPPORT always, so that the case where gf2x_cv_cc_supports_sse2/pclmul is undefined does not occur, with enable_sse2=no and enable_pclmul=no as appropriate.

comment:59 Changed 5 years ago by emassop

  • Branch changed from u/jpflori/ticket/15316 to u/emassop/ticket/15316

comment:60 Changed 5 years ago by emassop

  • Authors changed from Jean-Pierre Flori to Jean-Pierre Flori, Erik Massop
  • Commit changed from a23168950a9901003604b85a765379d24ac63fb3 to 3ca3e7b9386475ccefea3f9f9c6c0a29c141190d
  • Status changed from needs_work to needs_review

By seperating enabling of sse2/pclmul support from checking for support, configure can now run CHECK_(SSE2|PCLMUL)_SUPPORT always, avoiding the case where $gfx_cv_cc_support_(sse|pclmul) is not defined.

I'm afraid the patches are all different again. I imported upstream svn and the upstream tarballs into a git repository, then applied the changes in several topic branches (cygwin, hardware-specific, sse2, and tr), merge --squash'ed those branches into single commits, updated autotooled files (running autoreconf and reverse applying autoreconf's effect on upstream tarball), and finally ran git format-patch to obtain the patch series.

comment:61 Changed 5 years ago by jpflori

It's not so easy to see what the last commit actually does and unfortunately I have no time to play around applying different sets of patches to the src we distribute, diff the result and play with strange archs, compilers.

So my question is: what dirs will be used on x86_64 when the compiler support emitting sse2 instructions and when it does not and when the blabla-hardware-blabla flag is passed?

comment:62 Changed 5 years ago by emassop

  • Status changed from needs_review to needs_work

With --enable-hardware-specific-code (the default), it depends on whether code with sse2/pclmul can be compiled and executed.

cpu execute sse2 execute pclmul dir
x86_64 yes yes x86_64_pclmul
x86_64 no yes x86_64_pclmul
x86_64 yes no x86_64
x86_64 no no generic64

With --disable-hardware-specific-code: generic64.

Hmm, according to the discussion above that latter should be x86_64 instead of generic64, right? I'll fiddle with this some more.

I'm thinking of having the options --enable-sse2 and --enable-pclmul with values "no"/"hostcpu"/"native"/"yes", with as default the value of --enable-hardware-specific-code which would itself defaults to "native". Value "hostcpu" would enable minimally for this $host_cpu (for x86_64 this would be sse2 yes and pclmul no). Value "native" would try to compile and execute. Having calculated the final values for --enable-sse2 and --enable-pclmul, sse2 and pclmul would be enabled based on these values. Configure would error out if the compiler lacks support.

The default directory would be selected based on $host_cpu and whether sse2 and pclmul are enabled. When the directory is forced, insufficient sse2/pclmul might be enabled and configure would error out.

comment:63 Changed 5 years ago by jpflori

That's basically upstream choice: use generic when the hardware flag is passed. From my discussion with Emmanuel, it's basically because they don't care. I (and Jeroen?) found it disappointing because the x86_64 spec includes sse2 and quickly and easily came up with our patches which on x86_64 tested if the compiler supports sse2 and if so use the x86_64 folder. (Note that this folder uses sse2 instruction, so you cannot use it if the compiler does not support sse2.)

comment:64 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:65 Changed 5 years ago by jpflori

I'm basically ok with Erik last changes.

I'll just clean up the git branch to current standards and give it positive review.

comment:66 Changed 5 years ago by jpflori

  • Branch changed from u/emassop/ticket/15316 to u/jpflori/ticket/15316
  • Commit changed from 3ca3e7b9386475ccefea3f9f9c6c0a29c141190d to 969de52313ee407d874e9f7b318111c144de534e
  • Reviewers changed from Jeroen Demeyer to Jeroen Demeyer, Jean-Pierre Flori
  • Status changed from needs_work to needs_review

New commits:

9ba330eMerge remote-tracking branch 'trac/develop' into ticket/15316
969de52Update git branch for gf2x SSE2 and Cygwin fixes.

comment:67 Changed 5 years ago by jpflori

  • Status changed from needs_review to positive_review

comment:68 Changed 5 years ago by vbraun

  • Branch changed from u/jpflori/ticket/15316 to 969de52313ee407d874e9f7b318111c144de534e
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.