Re: Bug#905834: mozjs60: m68k patch from mozjs52 needs updating and re-testing?
On 9/27/18 3:07 PM, Simon McVittie wrote:
> Is this in the form you would send it upstream? I'd prefer it in the
> form of the individual patches you'd send upstream, rather than one
> big patch per architecture - that'll mean you don't have to prepare two
> different versions, and I don't have to discard the whole thing when
> part of it doesn't apply cleanly. Multiple patches on an email are fine,
> or so is a merge request on https://salsa.debian.org/gnome-team/mozjs60
> if that's easier for you.
I don't think that upstream would merge this. They don't like these
__attribute__((aligned(4))) statements. I also don't know what the
upstream development project for mozjs would be.
> Is <https://bugzilla.mozilla.org/show_bug.cgi?id=1325771> the right
> upstream bug reference for all this?
Yes, it's basically an updated set of these patches:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1325771
Mozilla upstream is, however, not going to merge any architecture
support for an architecture which doesn't support Rust yet (it's
a work-in-progress for m68k though).
> Does it work, and do the tests pass? In mozjs52, you said something failed
> with message "SSN_pattern - 8 = 8 expected: NaN", but I'm not sure which
> one: presumably js/src/tests/non262/RegExp/RegExp_object.js? Does that
> one still fail?
>
> In mozjs52 I ignored the test result completely for some architectures,
> but in mozjs60 I've switched to ignoring individual test failures, so that
> we can still distinguish between "mostly works" and "completely broken"
> even in the presence of isolated architecture-specific failures (which
> has already saved us from shipping broken s390x binaries that fail 80%
> of their tests).
>
> If a small number of tests fail (indicating that there are minor
> architecture-specific bugs, but the library generally works) please
> patch either js/src/tests/jstests.list or the first line of the
> tests themselves to mark the test as "skip-if" or "random-if" on m68k,
> similar to debian/patches/tests-Expect-a-test-to-fail-on-armel.patch and
> debian/patches/tests-Expect-some-floating-point-tests-to-fail-on-i386.patch;
> or let me know which ones are failing and I'll add appropriate annotations.
Again, in ports it's really not so important whether something works
perfectly or not. None of the packages that are in ports are going
to be released anytime soon and the people who take care of Debian
Ports know of all issues.
>> -} JS_HAZ_GC_THING;
>> +} JS_HAZ_GC_THING __attribute__ ((aligned(4)));
>
> This is in architecture-independent code, but seems harmless.
> I see you had some disputes over how best to fix alignments on
> <https://bugzilla.mozilla.org/show_bug.cgi?id=1325771>, but in Debian
> we only care about gcc (and maybe clang) so diverging from what you
> upstreamed by using this gcc-specific implementation seems fine.
The same part was used in the mozjs52 patch.
>> +static constexpr size_t sizemax(size_t a, size_t b)
>> +{
>> + return (a > b) ? a : b;
>> +}
>> +
>> INSTANTIATE(int, int, prim1, 2 * sizeof(int));
>> -INSTANTIATE(int, long, prim2, 2 * sizeof(long));
>> +INSTANTIATE(int, long, prim2, sizeof(long) + sizemax(sizeof(int), alignof(long)));
>>
>> struct EmptyClass { explicit EmptyClass(int) {} };
>> struct NonEmpty { char mC; explicit NonEmpty(int) {} };
>>
>> INSTANTIATE(int, EmptyClass, both1, sizeof(int));
>> -INSTANTIATE(int, NonEmpty, both2, 2 * sizeof(int));
>> +INSTANTIATE(int, NonEmpty, both2, sizeof(int) + alignof(int));
>> INSTANTIATE(EmptyClass, NonEmpty, both3, 1);
>
> https://bug1325771.bmoattachments.org/attachment.cgi?id=8831556
> makes it a lot clearer why this is correct, so I'd really prefer to
> apply the individual patches with their justifications intact.
Upstreaming such patches is already hard enough. Please don't make the
providing patches in Debian hard as well. I understand your arguments,
but brushing up patches so that they end up in the best possible form
that a package maintainer wants is a lot of work which is why I prefer
simple patches. Feel free to pull the patch apart.
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - glaubitz@debian.org
`. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Reply to: