first take at libgdiplus #45404

haf · 2015-10-27T22:11:37Z

Works with http://accord-framework.net/ for a number of tests... However; http://sourceforge.net/p/giflib/code/ci/ce02f85e6ed1e1455a4aea9ffa68a3e2b8f8a4b1/ and rainycape/magick#21 seem to hit us. It's moved to line 117 here http://sourceforge.net/p/giflib/code/ci/ce02f85e6ed1e1455a4aea9ffa68a3e2b8f8a4b1/tree/util/rgb2gif.c

This might be a better solution for OpenRA/OpenRA#6413

MikeMcQuaid · 2015-10-27T22:25:36Z

Library/Formula/libgdiplus.rb

We can't have this depend on X11, unfortunately; we can't assume users have it installed.

haf · 2015-10-27T22:28:29Z

@MikeMcQuaid how about when compiling like now?

haf · 2015-10-27T22:28:56Z

Another question: libgdiplus would prefer a pre 4.2 version of giflib – can I specify that somewhere?

MikeMcQuaid · 2015-10-27T22:36:23Z

Unfortunately not; it needs to use the newest version of glib or vendor it as a resource.

haf · 2015-10-27T22:38:45Z

Well, OK, I'd say this is more with regards to the libgdiplus project.

@directhex – could you use line the QuantizeBuffer function from http://sourceforge.net/p/giflib/code/ci/ce02f85e6ed1e1455a4aea9ffa68a3e2b8f8a4b1/ (a bit down in the util/rgb2gif.c file) instead of from the lib/gif_lib.h file?

haf · 2015-10-27T22:41:56Z

@directhex – actually it seems that an existing PR solves this issue: https://github.com/mono/libgdiplus/pull/32/files#diff-7c18b451e587fd18f4ded5380d98b15dR855 /cc @Mailaender

DomT4 · 2015-10-27T23:20:15Z

Library/Formula/libgdiplus.rb

Need nothing after head "https://github.com/mono/libgdiplus.git" here. Everything else is the assumed default.

Mailaender · 2015-10-28T05:20:13Z

Yeah, we have to patch this in @openSUSE for months now. I upstreamed it, adapted it according to the code review. No idea why mono/libgdiplus#32 hasn't been merged yet. Shouldn't be controversial.

haf · 2015-10-28T19:09:33Z

Ready to merge? The build is failing on some internal error in the audit command and the PR discussed above was successfully merged.

MikeMcQuaid · 2015-10-28T22:54:01Z

Library/Formula/libgdiplus.rb

We'll need a stable release before we can add this to Homebrew.

You mean you need a stable location to download the bytes from, because this code is both stable and released. How do you prefer it hosted and what precisely are your requirements to allow it to be included in homebrew? Is not github a stable place to download it from?

We need a stable, tagged release.

You should be clear about what you need so we can move forward from this back-and-forth:

So you need a git tag in the linked repo? Or you can take a tag in any cloned repo? Are you saying you need the official mono sponsor, Xamarin, to tag it? Or isn't the code base OpenSUSE uses (this particular commit) good enough? Or good enough if it's git-tagged? Does it matter this code-base hasn't been changed for basically three quarters of a year and that many a mono release has been built from it; many stable official releases. Or isn't it stable because I say so because I've verified that it is on my side? Please be specific.

Our documentation is clear.

So you need a git tag in the linked repo?

Yes.

Are you saying you need the official mono sponsor, Xamarin, to tag it?

Yes.

Thanks.

@directhex / Jo, Could you please tag the currently merged state in the git repository? Thank you. :)

MikeMcQuaid · 2015-10-30T21:25:20Z

Ready to merge?

No, it needs to not rely on X11. Additionally (repeated above from @xu-cheng) I think it would be better if you could make this a resource on the Mono formula after it no longer relies on X11 and has an official upstream tag.

haf · 2015-10-30T21:27:11Z

It relies on X11, that's how the software is built. And I've already followed @xu-cheng 's advice and removed the dependency on mono.

Just to make it clear; this is a piece of software that does graphics and X11 is closely related to graphics on unix.

The software doesn't drop dependencies because you declare it must, Mike. Could you explain your thinking?

dunn · 2015-10-31T03:40:27Z

We just don't allow software with hard X11 dependencies in the core repository. We have a homebrew-x11 tap for that.

haf · 2015-10-31T08:08:15Z

So let us merge to the tap instead then.

haf · 2015-10-31T08:09:23Z

But when you say hard, what do you mean? 99%of the usage of this module is not dependent on x11, and works fine without it.

rakkesh · 2015-10-31T08:25:09Z

@haf depends_on :x11 => :build is a hard dependency. If it was, depends_on :x11 => :optional, it would have been accepted in Homebrew core repository.

As @dunn has mentioned, you can open a PR over at https://github.com/Homebrew/homebrew-x11/pulls.

haf · 2015-10-31T08:46:09Z

It seems there are ifdefs removing the need to use X11:

./src/graphics.c:#ifdef CAIRO_HAS_XLIB_SURFACE and
./src/gdiplus-private.h: #ifdef CAIRO_HAS_XLIB_SURFACE. However, there's an explicit check in ./configure: configure: error: "Failed to compile with X11/Xlib.h include. You must fix your compiler paths"; can you avoid the check and the code?

rakkesh · 2015-10-31T09:13:40Z

You could use a patch to test it but I am not sure if this going to be accepted unless you can justify.

Remove the lines for X11 check from configure.ac, generate a diff and apply it in the formula.

haf · 2015-10-31T10:49:56Z

If I do it it should be accepted, otherwise I just should not do it. I am time constrained. I was asked to create a PR for this lib, but the discussion here is not encouraging. I don't give flying rats about X11 personally, I want to use Accord.Net on OS X, that's where I'm coming from.

rakkesh · 2015-10-31T11:04:10Z

I am sorry for digressing on this thread. Please follow @dunn's comment above, and make a PR in homebrew-x11 tap. That should be resolved quicker.

haf · 2015-10-31T11:11:56Z

I'm just going to leave this here with you guys and check back next weekend. Feel free to do what you want with it, I have the code which covers my own use-case.

MikeMcQuaid · 2015-11-01T02:10:39Z

You could use a patch to test it but I am not sure if this going to be accepted unless you can justify.

The justification is: OS X does not ship with X11 and this library doesn't seem to need X11 at runtime.

haf · 2015-11-02T16:14:15Z

So that means you'll accept patch to the library that removes the check for x11.h?

haf · 2015-11-02T19:31:17Z

So I've created a patch from https://github.com/mono/libgdiplus/pull/35.diff that seems to make it all compile fine without x11. However, the diff file doesn't work, and I haven't been google my way to a working diff file. The archaic binary that is patch talks about fuzz and chunks, but doesn't tell me what's wrong.

haf · 2015-11-04T09:36:52Z

ping @MikeMcQuaid

dunn · 2015-11-04T14:53:12Z

Inline patches are sometimes a bit fussy; try a patch do block as in https://github.com/Homebrew/homebrew/blob/master/Library/Formula/bash.rb#L16-L19

MikeMcQuaid · 2015-11-05T00:19:10Z

A few audit failures here: http://bot.brew.sh/job/Homebrew%20Pull%20Requests/35834/version=el_capitan/testReport/junit/brew-test-bot/el_capitan/audit_libgdiplus___strict___online/

With the patch either try a patch do block and if that doesn't work you'll probably need to manually patch the source as perhaps the upstream patch doesn't apply to this stable version.

bfontaine · 2015-12-16T14:33:45Z

Ping?

haf · 2015-12-17T10:21:30Z

I just don't know how to "manually patch the source"... I don't normally work with patches.

bfontaine · 2015-12-17T10:26:19Z

Run brew install -i --git libgdiplus; edit the source to fix the issues; run git diff | pbcopy; exit the shell. In the formula, at its very end add __END__ then paste the patch just after it. Add patch :DATA at the beginning of the formula to apply it. Tell me if that’s not clear/you need help.

bfontaine · 2016-01-15T14:26:15Z

Anything new here?

haf · 2016-01-15T16:26:01Z

It's slightly cyclic for us, what we work on. We're moving towards graphics again in a week or two and then we're going to finish this PR. Sorry about the time taken, but we have not forgotten it! =)

DomT4 · 2016-03-11T00:44:15Z

How's this going?

haf · 2016-03-11T07:52:20Z

I had an intern try to do the patches like people here want but he didn't succeed either.

bfontaine · 2016-04-10T10:46:31Z

Gentle ping 😉

apjanke · 2016-05-03T18:12:34Z

Closing this due to inactivity/lack of response. If you'd still like to pursue this, feel free to respond to the previous comments/questions and re-open on the new homebrew/core repo.

haf · 2016-05-03T18:26:42Z

Instead of closing, how about you fix the patch? You seem to know how to do it.

haf force-pushed the libgdiplus branch from 8cac138 to 921e95e Compare October 27, 2015 22:12

MikeMcQuaid reviewed Oct 27, 2015
View reviewed changes

Library/Formula/libgdiplus.rb Outdated

Copy link

Member

MikeMcQuaid Oct 27, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't have this depend on X11, unfortunately; we can't assume users have it installed.

haf force-pushed the libgdiplus branch from 921e95e to 5f1f50f Compare October 27, 2015 22:27

first take at libgdiplus

5f1f50f

DomT4 reviewed Oct 27, 2015
View reviewed changes

haf force-pushed the libgdiplus branch from d0ddc3e to b715fe6 Compare October 28, 2015 19:08

MikeMcQuaid reviewed Oct 28, 2015
View reviewed changes

hash rockets ftw

3456067

haf force-pushed the libgdiplus branch from b715fe6 to 3456067 Compare October 30, 2015 13:08

making it a mono resource, but dunno how to specify its deps

1b57e66

trying to create a diff file

d1a0fce

update mono formula to match

fe0f945

DomT4 mentioned this pull request Dec 7, 2015

libgdiplus: Needed for complete mono installation on os x #45393

Closed

bfontaine added the needs response label Dec 16, 2015

bfontaine added new formula and removed needs response labels Dec 17, 2015

DomT4 added the needs response label Dec 19, 2015

ghost removed the needs response label Apr 10, 2016

apjanke modified the milestone: Clear out Legacy May 3, 2016

apjanke closed this May 3, 2016

Homebrew locked and limited conversation to collaborators Jul 10, 2016

Uh oh!

first take at libgdiplus #45404

first take at libgdiplus #45404

Uh oh!

Conversation

haf commented Oct 27, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haf commented Oct 27, 2015

Uh oh!

haf commented Oct 27, 2015

Uh oh!

MikeMcQuaid commented Oct 27, 2015

Uh oh!

haf commented Oct 27, 2015

Uh oh!

haf commented Oct 27, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mailaender commented Oct 28, 2015

Uh oh!

haf commented Oct 28, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MikeMcQuaid commented Oct 30, 2015

Uh oh!

haf commented Oct 30, 2015

Uh oh!

dunn commented Oct 31, 2015

Uh oh!

haf commented Oct 31, 2015

Uh oh!

haf commented Oct 31, 2015

Uh oh!

rakkesh commented Oct 31, 2015

Uh oh!

haf commented Oct 31, 2015

Uh oh!

rakkesh commented Oct 31, 2015

Uh oh!

haf commented Oct 31, 2015

Uh oh!

rakkesh commented Oct 31, 2015

Uh oh!

haf commented Oct 31, 2015

Uh oh!

MikeMcQuaid commented Nov 1, 2015

Uh oh!

haf commented Nov 2, 2015

Uh oh!

haf commented Nov 2, 2015

Uh oh!

haf commented Nov 4, 2015

Uh oh!

dunn commented Nov 4, 2015

Uh oh!

MikeMcQuaid commented Nov 5, 2015

Uh oh!

bfontaine commented Dec 16, 2015

Uh oh!

haf commented Dec 17, 2015

Uh oh!

bfontaine commented Dec 17, 2015

Uh oh!

bfontaine commented Jan 15, 2016