[Update] Ipe 7.2.9

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[Update] Ipe 7.2.9

Alessandro DE LAURENZIS-3
Dear ports@ readers,

Here is a proposal for updating Ipe (the extensible drawing editor) to
the latest release.

The version in the tree is pretty old (7.1.2, 16 Jan 2012) and I do not
see any blocking point to jump on the current one, since no package
depends on it.


What's new upstream
===================

Too many things, actually; most notable ones:
- Migrated to Qt5;
- Requires a C++11 compiler;
- Using Lua 5.3.

For a complete list of changes, see news.txt.


What's new in the port
======================

- Project is no more hosted on sourceforge; HOMEPAGE and MASTER_SITES
updated;

- WANTLIB, MODULES and LIB_DEPENDS updated to reflect the changes in QT
and LUA versions (the latter forced to 5.3 through MODLUA_VERSION);

- added JPEG_CFLAGS and DL_LIBS to MAKE_ENV;

- post-install target no more needed;

- PLIST updated;

- patches have been reshuffled to reflect code changes;

- two old patches (patch-src_ipelib_ipebase_cpp and
patch-src_ipelib_ipepdfparser_cpp) are no more needed, since upstream
code now include cstdlib;

- I added patch-src_ipelib_ipeplatform_cpp to play with strtod_l, which
is not supported by OpenBSD;

Tested on amd64; I'm attaching a complete tarball, since the long
recursive diff seems a bit confusing to me.

I'm also available to take the maintainership, in case Edd had low/no
interest in this tool (I use it extensively).

All the best

--
Alessandro DE LAURENZIS
[mailto:[hidden email]]
Web: http://www.atlantide.t28.net
LinkedIn: https://www.linkedin.com/in/delaurenzis/

ipe.tar.gz (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Update] Ipe 7.2.9

Edd Barrett-3
On Sun, Feb 03, 2019 at 05:10:07PM +0100, Alessandro DE LAURENZIS wrote:
> Dear ports@ readers,
>
> Here is a proposal for updating Ipe (the extensible drawing editor) to the
> latest release.

Thanks, I'm happy to review this, but can you send your update as a
unified diff from cvs?

`cvs add` and `cvs rm` files as necessary (you should only have M, R and
A lines in the output of `cvs -n up -dP`) and then do:

cvs diff -uNp > /tmp/ipe.diff

We only do tarballs for new ports, and diffs are easier to review.

Thanks

--
Best Regards
Edd Barrett

http://www.theunixzoo.co.uk

Reply | Threaded
Open this post in threaded view
|

Re: [Update] Ipe 7.2.9

Alessandro DE LAURENZIS-3
Hello Edd,

thanks for your time to revisit this port; please find enclosed the
unified diff.

All the best

On 2/4/19 12:11 PM, Edd Barrett wrote:

> On Sun, Feb 03, 2019 at 05:10:07PM +0100, Alessandro DE LAURENZIS wrote:
>> Dear ports@ readers,
>>
>> Here is a proposal for updating Ipe (the extensible drawing editor) to the
>> latest release.
>
> Thanks, I'm happy to review this, but can you send your update as a
> unified diff from cvs?
>
> `cvs add` and `cvs rm` files as necessary (you should only have M, R and
> A lines in the output of `cvs -n up -dP`) and then do:
>
> cvs diff -uNp > /tmp/ipe.diff
>
> We only do tarballs for new ports, and diffs are easier to review.
>
> Thanks
>

--
Alessandro DE LAURENZIS
[mailto:[hidden email]]
Web: http://www.atlantide.t28.net
LinkedIn: https://www.linkedin.com/in/delaurenzis/

ipe.diff (31K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Update] Ipe 7.2.9

Alessandro DE LAURENZIS-3
Hello Edd,

any comments on this?

Thanks

Il 4 febbraio 2019 20:33:08 CET, Alessandro DE LAURENZIS <[hidden email]> ha scritto:

>Hello Edd,
>
>thanks for your time to revisit this port; please find enclosed the
>unified diff.
>
>All the best
>
>On 2/4/19 12:11 PM, Edd Barrett wrote:
>> On Sun, Feb 03, 2019 at 05:10:07PM +0100, Alessandro DE LAURENZIS
>wrote:
>>> Dear ports@ readers,
>>>
>>> Here is a proposal for updating Ipe (the extensible drawing editor)
>to the
>>> latest release.
>>
>> Thanks, I'm happy to review this, but can you send your update as a
>> unified diff from cvs?
>>
>> `cvs add` and `cvs rm` files as necessary (you should only have M, R
>and
>> A lines in the output of `cvs -n up -dP`) and then do:
>>
>> cvs diff -uNp > /tmp/ipe.diff
>>
>> We only do tarballs for new ports, and diffs are easier to review.
>>
>> Thanks
>>
>
>
>--
>Alessandro DE LAURENZIS
>[mailto:[hidden email]]
>Web: http://www.atlantide.t28.net
>LinkedIn: https://www.linkedin.com/in/delaurenzis/

--
Inviato dal mio dispositivo Android con K-9 Mail. Perdonate la brevità.
Reply | Threaded
Open this post in threaded view
|

Re: [Update] Ipe 7.2.9

Edd Barrett-3
In reply to this post by Alessandro DE LAURENZIS-3
Hi Alessandro,

On Mon, Feb 04, 2019 at 08:33:08PM +0100, Alessandro DE LAURENZIS wrote:
> thanks for your time to revisit this port; please find enclosed the unified
> diff.

Seems like there's a problem with the diff:

---8<---
$ patch -CEsp0 < /tmp/ipe.diff
...
Patching file patches/patch-src_common_mak using Plan A...
Hunk #1 failed at 1.
...
--->8---

Then here's a few more comments, from just reading the diff:

> -V = 7.1.2
> +VERSION = 7.2
> +V = ${VERSION}.9

Having a V and a VERSION is a little confusing.

How about MAJOR_V and V, for the major version and the full version?

> +MODLUA_VERSION =        5.3

Is this a hard requirement? It'd be better to use the default ports
version (which is currently 5.1, but I think you can just remove this
line if 5.1 is OK).

> + DL_LIBS=""

^ What's this for?

> +-LUA_PACKAGE   ?= lua5.3
> ++LUA_PACKAGE   ?= lua53

I think we should try to substitute that lua version in, so that it
doesn't break if we change the lua version.

> +   -- On Linux, you could use: prefs.keyboard = "onboard &"
> +-  prefs.keyboard = nil
> ++  prefs.keyboard = "xvkbd &"

Are these defaults, or are we hard-coding an application here?

By the way, I'm happy for you to take MAINTAINER if you'd like to.

Cheers

--
Best Regards
Edd Barrett

http://www.theunixzoo.co.uk

Reply | Threaded
Open this post in threaded view
|

Re: [Update] Ipe 7.2.9

Alessandro DE LAURENZIS-3
Hello Edd,

On 15/02/2019 21:31, Edd Barrett wrote:

> Hi Alessandro,
>
> On Mon, Feb 04, 2019 at 08:33:08PM +0100, Alessandro DE LAURENZIS wrote:
>> thanks for your time to revisit this port; please find enclosed the unified
>> diff.
>
> Seems like there's a problem with the diff:
>
> ---8<---
> $ patch -CEsp0 < /tmp/ipe.diff
> ...
> Patching file patches/patch-src_common_mak using Plan A...
> Hunk #1 failed at 1.
> ...
> --->8---
Weird... I can apply it here without issues.

>
> Then here's a few more comments, from just reading the diff:
>
>> -V = 7.1.2
>> +VERSION = 7.2
>> +V = ${VERSION}.9
>
> Having a V and a VERSION is a little confusing.
>
> How about MAJOR_V and V, for the major version and the full version?
>
Sure. Done.

>> +MODLUA_VERSION =        5.3
>
> Is this a hard requirement? It'd be better to use the default ports
> version (which is currently 5.1, but I think you can just remove this
> line if 5.1 is OK).

That's definitely an hard requirement; package doesn't compile with LUA
versions older that 5.3

>
>> + DL_LIBS=""
>
> ^ What's this for?

In 2 files (${WRKSRC}/src/common.mak and ${WRKSRC}/src/snapcraft.mak)
there is the following assignment:

DL_LIBS         ?= -ldl

so either we patch them or define an empty DL_LIBS in the port's
Makefile, I think. Or maybe there are better ways to manage that?

>
>> +-LUA_PACKAGE   ?= lua5.3
>> ++LUA_PACKAGE   ?= lua53
>
> I think we should try to substitute that lua version in, so that it
> doesn't break if we change the lua version.
>

Done using ${LUA_DEP_VERSION} and ${SUBST_CMD} in pre-build target.

>> +   -- On Linux, you could use: prefs.keyboard = "onboard &"
>> +-  prefs.keyboard = nil
>> ++  prefs.keyboard = "xvkbd &"
>
> Are these defaults, or are we hard-coding an application here?

This is the same approach as before, I didn't change it. And yes, we're
hard-coding an application... I cannot help much here, suggestions?

>
> By the way, I'm happy for you to take MAINTAINER if you'd like to.
>

MAINTAINER line updated.


New unified diff attached (with a slightly tweaking of line spaces and
tabs in the port's Makefile). Please double check if you can apply it
cleanly.

Thanks a lot

--
Alessandro DE LAURENZIS
[mailto:[hidden email]]
Web: http://www.atlantide.t28.net
LinkedIn: https://www.linkedin.com/in/delaurenzis/

ipe.diff (31K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Update] Ipe 7.2.9

Stuart Henderson
On 2019/02/16 17:14, Alessandro DE LAURENZIS wrote:

> Hello Edd,
>
> On 15/02/2019 21:31, Edd Barrett wrote:
> > Hi Alessandro,
> >
> > On Mon, Feb 04, 2019 at 08:33:08PM +0100, Alessandro DE LAURENZIS wrote:
> > > thanks for your time to revisit this port; please find enclosed the unified
> > > diff.
> >
> > Seems like there's a problem with the diff:
> >
> > ---8<---
> > $ patch -CEsp0 < /tmp/ipe.diff
> > ...
> > Patching file patches/patch-src_common_mak using Plan A...
> > Hunk #1 failed at 1.
> > ...
> > --->8---
>
> Weird... I can apply it here without issues.

I didn't try the old one, but this one does apply ok.

> >
> > Then here's a few more comments, from just reading the diff:
> >
> > > -V = 7.1.2
> > > +VERSION = 7.2
> > > +V = ${VERSION}.9
> >
> > Having a V and a VERSION is a little confusing.
> >
> > How about MAJOR_V and V, for the major version and the full version?
> >
>
> Sure. Done.

You can just set V=7.2.9 and use

MASTER_SITES= https://dl.bintray.com/otfried/generic/ipe/${V:R}/

> > > +MODLUA_VERSION =        5.3
> >
> > Is this a hard requirement? It'd be better to use the default ports
> > version (which is currently 5.1, but I think you can just remove this
> > line if 5.1 is OK).
>
> That's definitely an hard requirement; package doesn't compile with LUA
> versions older that 5.3

Please set MODLUA_SA=Yes.

> > > + DL_LIBS=""
> >
> > ^ What's this for?
>
> In 2 files (${WRKSRC}/src/common.mak and ${WRKSRC}/src/snapcraft.mak) there
> is the following assignment:
>
> DL_LIBS         ?= -ldl
>
> so either we patch them or define an empty DL_LIBS in the port's Makefile, I
> think. Or maybe there are better ways to manage that?

It's fine like you have it.

> >
> > > +-LUA_PACKAGE   ?= lua5.3
> > > ++LUA_PACKAGE   ?= lua53
> >
> > I think we should try to substitute that lua version in, so that it
> > doesn't break if we change the lua version.
> >
>
> Done using ${LUA_DEP_VERSION} and ${SUBST_CMD} in pre-build target.

You have some bogus ${LUA_DEP_VERSION} in the PLIST. If you fix them up
you should be able to stop it readding them with

UPDATE_PLIST_ARGS += -i LUA_DEP_VERSION

Reply | Threaded
Open this post in threaded view
|

Re: [Update] Ipe 7.2.9

Alessandro DE LAURENZIS-3
Hi Stuart,

On 16/02/2019 18:37, Stuart Henderson wrote:
> You can just set V=7.2.9 and use
>
> MASTER_SITES= https://dl.bintray.com/otfried/generic/ipe/${V:R}/
>

Done.

> Please set MODLUA_SA=Yes.
>

Done.

> You have some bogus ${LUA_DEP_VERSION} in the PLIST. If you fix them up
> you should be able to stop it readding them with
>
> UPDATE_PLIST_ARGS += -i LUA_DEP_VERSION
>

Just added UPDATE_PLIST_ARGS and the bogus ${MODLUA_DEP_VERSION} (there
was only one) has gone.

While there:

- yet another tabs tweak;
- added IPEDOCDIR to MAKE_FLAGS (it was already in FAKE_FLAGS),
otherwise the "Ipe manual" item in the help menu doesn't work.

Updated diff attached.

--
Alessandro DE LAURENZIS
[mailto:[hidden email]]
Web: http://www.atlantide.t28.net
LinkedIn: https://www.linkedin.com/in/delaurenzis/

ipe.diff (32K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Update] Ipe 7.2.9

Alessandro DE LAURENZIS-3
Stuart, Edd,

On 16/02/2019 20:31, Alessandro DE LAURENZIS wrote:

[...]
> While there:
>
> - yet another tabs tweak;
> - added IPEDOCDIR to MAKE_FLAGS (it was already in FAKE_FLAGS),
> otherwise the "Ipe manual" item in the help menu doesn't work.
>

Following Stuart's suggestion not to mix port's update and style tiding,
I'm attaching an updated diff with no whitespace changes which I hope is
more easy to read.

Please let me know if it is ok.

All the best

--
Alessandro DE LAURENZIS
[mailto:[hidden email]]
Web: http://www.atlantide.t28.net
LinkedIn: https://www.linkedin.com/in/delaurenzis/

ipe.diff (31K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Update] Ipe 7.2.9

Stuart Henderson
On 2019/02/24 10:14, Alessandro DE LAURENZIS wrote:

> Stuart, Edd,
>
> On 16/02/2019 20:31, Alessandro DE LAURENZIS wrote:
>
> [...]
> > While there:
> >
> > - yet another tabs tweak;
> > - added IPEDOCDIR to MAKE_FLAGS (it was already in FAKE_FLAGS),
> > otherwise the "Ipe manual" item in the help menu doesn't work.
> >
>
> Following Stuart's suggestion not to mix port's update and style tiding, I'm
> attaching an updated diff with no whitespace changes which I hope is more
> easy to read.
>
> Please let me know if it is ok.

> +HOMEPAGE = http://ipe.otfried.org/

nitpicking, http->https

> diff -u -p -u -p -r1.1.1.1 patch-src_common_mak

The diff that came out of the email process doesn't apply because the
patch file in-tree has MSDOS line endings.

Regenerated diff attached (gzipped, so it should survive email).
OK with you Edd?


ipe.diff.gz (8K) Download Attachment