Closed Bug 1233966 Opened 9 years ago Closed 8 years ago

gfx/skia/skia/src/ports/SkTime_Unix.cpp:27:60: error: invalid operands of types 'char*(int, int)' and 'int' to binary 'operator/'

Categories

(Core :: Graphics, defect)

Unspecified
FreeBSD
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: jbeich, Assigned: lsalzman)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file, 1 obsolete file)

BSDs don't have |extern long timezone| from XSI but a function that probably predates it. OS X has either selected via __DARWIN_UNIX03 (default on 64bit).

In file included from gfx/skia/Unified_cpp_gfx_skia24.cpp:101:
gfx/skia/skia/src/ports/SkTime_Unix.cpp:27:58: error: invalid
      operands to binary expression ('char *(*)(int, int)' and 'int')
        dt->fTimeZoneMinutes = SkToS16(offset - timezone / 60);
                                                ~~~~~~~~ ^ ~~
gfx/skia/skia/include/core/SkTypes.h:249:36: note: expanded from
      macro 'SkToS16'
    #define SkToS16(x)  ((int16_t)(x))
                                   ^
1 error generated.

https://man.freebsd.org/timezone/3
FreeBSD forbids using <malloc.h> while DragonFly lacks the file.

In file included from gfx/skia/Unified_cpp_gfx_skia9.cpp:74:
In file included from gfx/skia/skia/src/core/SkVarAlloc.cpp:14:
In file included from obj-*/dist/system_wrappers/malloc.h:3:
/usr/include/malloc.h:3:2: error: "<malloc.h> has been replaced by <stdlib.h>"
#error "<malloc.h> has been replaced by <stdlib.h>"
 ^
1 error generated.

https://man.freebsd.org/malloc_usable_size/3
Note, I did read gfx/skia/README_COMMITTING but cannot follow. It requires submitting to upstream CLA which in turn requires a Google Account and that cannot be signed up for via Tor.
Comment on attachment 8700312 [details]
MozReview Request: Bug 1233966 - Unbreak build on BSDs after bug 1082598. r?lsalzman

These should really be dealt with upstream.

Just file an upstream bug report on their issue tracker at https://bugs.chromium.org/p/skia/issues/list

If the CLA is a problem, then at least explain the issues to them and suggest how to fix them, which does not in and of itself require signing the CLA. Also, they might have different suggestions as to how to resolve the problem that may or may not require your actual patches.

However, I would recommend filing your patches as attachments in the upstream issue report, and although it might get addressed a bit slower, they will still look into it, I believe.
Attachment #8700312 - Flags: review?(lsalzman) → review-
Landry, can you confirm comment 0 affects OpenBSD as well? comment 1 is confirmed by

http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/www/chromium/patches/patch-third_party_skia_src_core_SkVarAlloc_cpp

(In reply to Lee Salzman [:lsalzman] from comment #4)
> Just file an upstream bug report on their issue tracker at
> https://bugs.chromium.org/p/skia/issues/list

Let me reiterate comment 3: I cannot follow. It's partially related to https://lists.torproject.org/pipermail/tor-talk/2012-October/025926.html except applies even to non-Google mail.

1. New issue
2. Create account
3. I prefer to use my current email address 
4. Prove you're not a robot
5. Swap exit nodes until it gives a human-readable captcha
6. Verify your account: Text message or Voice Call
7. Give up due to lack of "Ask me later" option

> If the CLA is a problem

Not in itself but as part of Google Account ecosystem. Requiring unpaid contributors to sacrifice privacy is rude, and also sad if Mozilla cannot liaison a one-off patch against the stuff it uses.

Feel free to claim attachment 8700312 [details], comment 0, comment 1 as your own and/or assume I've agreed to Google's CLA out-of-band.

> then at least explain the issues to them and suggest how to fix them,

Before I can do that there're more issues blocking upstream build.

  $ gmake
  /path/to/skia/gyp_skia --no-parallel -G config=Debug
  GYP_GENERATORS environment variable not set, using default
  GYP_GENERATORS is "ninja"
  Updating projects from gyp files...
  ninja -C out/Debug most
  ninja: Entering directory `out/Debug'
  ninja: error: '../../third_party/yasm/config/freebsd/Makefile', needed by 'obj/gyp/yasm.gen/third_party/yasm/module.c', missing and no known rule to make it
  Makefile:97: recipe for target 'most' failed

  $ ln -s openbsd third_party/yasm/config/freebsd
  $ gmake
  [313/1462] LINK genperf
  FAILED: cc -m64 -o genperf -Wl,--start-group obj/third_party/externals/yasm/source/patched-yasm/tools/genperf/genperf.genperf.o obj/third_party/externals/yasm/source/patched-yasm/tools/genperf/genperf.perfect.o obj/gyp/libgenperf_libs.a -Wl,--end-group
  /usr/bin/ld:obj/gyp/libgenperf_libs.a: file format not recognized; treating as linker script
  /usr/bin/ld:obj/gyp/libgenperf_libs.a:1: syntax error
  cc: error: linker command failed with exit code 1 (use -v to see invocation)

  $ find out -name '*.a' -delete
  $ export PATH=/usr/bin:$PATH 
  $ gmake
  In file included from ../../src/ports/SkFontMgr_fontconfig_factory.cpp:9:
  ../../include/ports/SkFontMgr_fontconfig.h:12:10: fatal error: 'fontconfig/fontconfig.h' file not found
  #include <fontconfig/fontconfig.h>
	   ^
  1 error generated.

  $ export CPATH=/usr/local/include LIBRARY_PATH=/usr/local/lib
  $ gmake
  In file included from ../../src/ports/SkFontHost_FreeType.cpp:15:
  ../../src/ports/SkFontHost_FreeType_common.h:18:10: fatal error: 'ft2build.h' file not found
  #include <ft2build.h>
	   ^
  1 error generated.

  $ export CPATH=$CPATH:/usr/local/include/freetype2
  $ gmake
  /usr/bin/ld: cannot find -ldl

  $ sed -i '' '/-ldl/d' gyp/ports.gyp
  $ gmake
  libskia_skgputest.a(skgputest.GrContextFactory.o): In function `GrContextFactory::getContextInfo(GrContextFactory::GLContextType, GrGLStandard, GrContextFactory::GLContextOptions)':
  ../../src/gpu/GrContextFactory.cpp:(.text+0xc5): undefined reference to `SkCreatePlatformGLContext(GrGLStandard)'
  c++: error: linker command failed with exit code 1 (use -v to see invocation)

  $ sed -i '' 's/linux/freebsd/' gyp/gpu.gyp
  $ gmake
  $ ./out/Debug/HelloWorld

DragonFly and NetBSD likely have even more errors.
Depends on: 1234494
I've been able to build m-i tip with only http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/www/chromium/patches/patch-third_party_skia_src_gpu_GrAutoLocaleSetter_h?rev=1.2&content-type=text/x-cvsweb-markup in my patch queue - didnt experience issues from comment 0 or 1.

And as i said in bug #1234494, i'm totally with jan here - having to deal with pushing back patches to skia is a hassle (i've already filed some bugs upstream in various chromium sub-projects without proper resolution), *and* the CLA/google crap is not acceptable. We have enough work with mozilla alone :)
Upstream bug report: https://bugs.chromium.org/p/skia/issues/detail?id=4736

The point is not to put an unreasonable burden on bug reporters, bug rather ensure we don't end up effectively forking Skia like we did with Cairo, which does put an unreasonable burden on us.

Further that since we don't officially test the *BSD configurations, having us serve as an intermediary would not be the most effective way to deal with the multiplicity of reported issues, nor are we able to verify that the issues were simply fixed - you both are in a better position to manage that.

If the CLA presents an unreasonable burden, then I would still recommend at least setting up an account to file the issues and communicate with them directly. I can only really play middle-man on this running comments back and forth, whereas, again, you folks have the best expertise on whether the issues have been adequately addressed to suit your needs.

So, the best outcomes will still come by having you directly interact with them upstream if you can help us out there.
(In reply to Landry Breuil (:gaston) from comment #6)
> didnt experience issues from comment 0 or 1.

Thanks. Only FreeBSD sticks to old timezone(3) API, nowadays. There's a downstream bug stalled for 14 years.
Whiteboard: [gfx-noted]
Just commented out the use of malloc.h since malloc_usable_size isn't currently used at all in SkVarAlloc.cpp anyway, and may be a bit questionable anyway since it is not abstracted with the rest of the sk_malloc implementation. Depending on combinations of build flags supplied to the Firefox configuration, which implementation of malloc you get in the end is a bit up in the air. The linker will probably figure the right thing out, although the prototype becomes somewhat irrelevant at that point. But, getting crazy deciding which header to use for a commented out assert is a bit of a headache... The only winning move is not to play.

The SkTime_Unix.cpp fixes from FreeBSD, I just took as-is.
Assignee: nobody → lsalzman
Attachment #8700312 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8702331 - Flags: review?(jmuizelaar)
Comment on attachment 8702331 [details] [diff] [review]
build fixes for FreeBSD

Re-tested to confirm this version also fixes my build.
Attachment #8702331 - Flags: feedback+
Attachment #8702331 - Flags: review?(jmuizelaar) → review+
We're not ready to check these in yet as we're still waiting on upstream to have a chance to investigate. Please be patient.
Keywords: checkin-needed
Sorry, was just wondering why my builds were still red, and wondered why this hadnt landed yet since it had been r+'ed... thanks for following up with upstream !
Okay, just going to land these as-is, since the upstream changes are a bit more extensive. Once we update to Skia m49+ we won't need these fixes anymore, so for now these will just let us work around the issue with minimal changes.
https://hg.mozilla.org/mozilla-central/rev/11affe05f061
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: