NEW games/cataclysm-dda

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

NEW games/cataclysm-dda

trondd-2

attachment0 (16K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: NEW games/cataclysm-dda

trondd-2
Tobias Ulmer <[hidden email]> wrote:

> On Sat, Oct 14, 2017 at 04:43:14PM -0400, trondd wrote:
>
> Hi trondd,
>
> no idea what happened, but I can't read your mail at all. It appears as
> one big binary in mutt.
>
> Could you re-send it to the list? I'd like to take a look

Whoops.  Attached the file instead of inlining it this time.

Tim.


cdda-20171013.tgz (16K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: NEW games/cataclysm-dda

Brian Callahan-3
Hi Tim --

On 10/22/17 11:38, trondd wrote:

> Tobias Ulmer <[hidden email]> wrote:
>
>> On Sat, Oct 14, 2017 at 04:43:14PM -0400, trondd wrote:
>>
>> Hi trondd,
>>
>> no idea what happened, but I can't read your mail at all. It appears as
>> one big binary in mutt.
>>
>> Could you re-send it to the list? I'd like to take a look
> Whoops.  Attached the file instead of inlining it this time.
>
> Tim.
>
I didn't really know how to go about talking it out, so here's a new
tarball with the complete port with my fixes. I'm compiling the no_x11
FLAVOR now. The SDL2 version didn't work for me (it launched a windows
but then had to be kill -9'd), but it's impossible for me to tell if it
is indeed broken or if it is my super old and shitty netbook that just
refuses to play nicely with it.

~Brian


cataclysm-dda.tgz (17K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: NEW games/cataclysm-dda

Brian Callahan-3

On 10/22/17 22:42, Brian Callahan wrote:

> Hi Tim --
>
> On 10/22/17 11:38, trondd wrote:
>> Tobias Ulmer <[hidden email]> wrote:
>>
>>> On Sat, Oct 14, 2017 at 04:43:14PM -0400, trondd wrote:
>>>
>>> Hi trondd,
>>>
>>> no idea what happened, but I can't read your mail at all. It appears as
>>> one big binary in mutt.
>>>
>>> Could you re-send it to the list? I'd like to take a look
>> Whoops.  Attached the file instead of inlining it this time.
>>
>> Tim.
>>
>
> I didn't really know how to go about talking it out, so here's a new
> tarball with the complete port with my fixes. I'm compiling the no_x11
> FLAVOR now. The SDL2 version didn't work for me (it launched a windows
> but then had to be kill -9'd), but it's impossible for me to tell if
> it is indeed broken or if it is my super old and shitty netbook that
> just refuses to play nicely with it.
>
> ~Brian
>

Update: the console version works fine for me. Maybe I just didn't wait
long enough for the SDL version to load.

Reply | Threaded
Open this post in threaded view
|

Re: NEW games/cataclysm-dda

trondd-2
On Mon, October 23, 2017 1:25 am, Brian Callahan wrote:

>
> On 10/22/17 22:42, Brian Callahan wrote:
>> Hi Tim --
>>
>> On 10/22/17 11:38, trondd wrote:
>>> Tobias Ulmer <[hidden email]> wrote:
>>>
>>>> On Sat, Oct 14, 2017 at 04:43:14PM -0400, trondd wrote:
>>>>
>>>> Hi trondd,
>>>>
>>>> no idea what happened, but I can't read your mail at all. It appears
>>>> as
>>>> one big binary in mutt.
>>>>
>>>> Could you re-send it to the list? I'd like to take a look
>>> Whoops.  Attached the file instead of inlining it this time.
>>>
>>> Tim.
>>>
>>
>> I didn't really know how to go about talking it out, so here's a new
>> tarball with the complete port with my fixes. I'm compiling the no_x11
>> FLAVOR now. The SDL2 version didn't work for me (it launched a windows
>> but then had to be kill -9'd), but it's impossible for me to tell if
>> it is indeed broken or if it is my super old and shitty netbook that
>> just refuses to play nicely with it.
>>
>> ~Brian
>>
>
> Update: the console version works fine for me. Maybe I just didn't wait
> long enough for the SDL version to load.
>

Thanks.  I'll take a look at your changes tonight, but quickly: Yes, it
takes a weirdly long time to start up.  Does this on OSX as well.

Reply | Threaded
Open this post in threaded view
|

Re: NEW games/cataclysm-dda

trondd-2
In reply to this post by Brian Callahan-3
On Sun, October 22, 2017 10:42 pm, Brian Callahan wrote:

>
> I didn't really know how to go about talking it out, so here's a new
> tarball with the complete port with my fixes. I'm compiling the no_x11
> FLAVOR now. The SDL2 version didn't work for me (it launched a windows
> but then had to be kill -9'd), but it's impossible for me to tell if it
> is indeed broken or if it is my super old and shitty netbook that just
> refuses to play nicely with it.
>
> ~Brian
>
>

I'll talk then. :)  I don't see any problems with your fixes, but I have
questions and want to understand them.  Thanks for taking the time to
review.


> BUILD_DEPENDS=          devel/astyle \

I saw your comments in the update to astyle.  I had ignored astyle because
I didn't think it was important for the port to care about upstream style
conformity.  I don't know that it'll ever be an issue as upstream does
'experimental' builds off master for linux, windows and OSX so I think
style issues will be caught.  But since we're building from development
source, would we want a style issue to fail the build and stop an update
to the port?


> MAKE_FLAGS= ...  CXX="${CXX}"
>
> pre-configure:
>        sed -i -e 's,-Os,${CXXFLAGS},g'
>  -e 's,$${LOCALBASE},${LOCALBASE},g' \
>                -e 's,-Werror,,g' ${WRKSRC}/Makefile

I'm guessing defining CXX, replacing '-Os' and removing '-Werror' are to
allow the pkg tools to control the build flags.  Or something more
specific about -O and -W?

Substituting LOCALBASE here I'm lost on.  I don't see where that was
targeting in the Makefile.  Anyway, why wouldn't it get substituted when
make was run?

Tim.

Reply | Threaded
Open this post in threaded view
|

Re: NEW games/cataclysm-dda

Brian Callahan-3
Hi Tim --

On 10/23/17 21:16, trondd wrote:

> On Sun, October 22, 2017 10:42 pm, Brian Callahan wrote:
>> I didn't really know how to go about talking it out, so here's a new
>> tarball with the complete port with my fixes. I'm compiling the no_x11
>> FLAVOR now. The SDL2 version didn't work for me (it launched a windows
>> but then had to be kill -9'd), but it's impossible for me to tell if it
>> is indeed broken or if it is my super old and shitty netbook that just
>> refuses to play nicely with it.
>>
>> ~Brian
>>
>>
> I'll talk then. :)  I don't see any problems with your fixes, but I have
> questions and want to understand them.  Thanks for taking the time to
> review.
>
>
>> BUILD_DEPENDS=          devel/astyle \
> I saw your comments in the update to astyle.  I had ignored astyle because
> I didn't think it was important for the port to care about upstream style
> conformity.  I don't know that it'll ever be an issue as upstream does
> 'experimental' builds off master for linux, windows and OSX so I think
> style issues will be caught.  But since we're building from development
> source, would we want a style issue to fail the build and stop an update
> to the port?

The build moves on even if astyle fails. I'm not wedded to keeping it as
a BDEP. You can remove the astyle lines from cdda's Makefile if you like.

>
>> MAKE_FLAGS= ...  CXX="${CXX}"
>>
>> pre-configure:
>>         sed -i -e 's,-Os,${CXXFLAGS},g'
>>   -e 's,$${LOCALBASE},${LOCALBASE},g' \
>>                 -e 's,-Werror,,g' ${WRKSRC}/Makefile
> I'm guessing defining CXX, replacing '-Os' and removing '-Werror' are to
> allow the pkg tools to control the build flags.  Or something more
> specific about -O and -W?

Correct. We want the pkg tools to control optimization flags.
As for -Werror, that's a flag used for testing, not for building
releases (unless you're damn sure you tested every possible combination
of every possible compiler that can build your software. And even
then...). Best to always get rid of it.

> Substituting LOCALBASE here I'm lost on.  I don't see where that was
> targeting in the Makefile.  Anyway, why wouldn't it get substituted when
> make was run?
>

Indeed you're right. It's a leftover from my trying something else. The
pre-configure routine can be reduced to:
pre-configure:
         sed -i -e 's,-Os,${CXXFLAGS},g' -e 's,-Werror,,g'
${WRKSRC}/Makefile

~Brian

Reply | Threaded
Open this post in threaded view
|

Re: NEW games/cataclysm-dda

trondd-2
Brian Callahan <[hidden email]> wrote:

>
> The build moves on even if astyle fails. I'm not wedded to keeping it as
> a BDEP. You can remove the astyle lines from cdda's Makefile if you like.
>

The build moves on if astyle is missing or too old.  It fails if there is
an actual style issue.  I would have leaned towards excluding it based on
reducing dependencies but since astyle doesn't bing in any more deps, I
left it in for completeness.

>
> Indeed you're right. It's a leftover from my trying something else. The
> pre-configure routine can be reduced to:
> pre-configure:
>          sed -i -e 's,-Os,${CXXFLAGS},g' -e 's,-Werror,,g'
> ${WRKSRC}/Makefile
>
> ~Brian

Cleaned that up and I guess this is the final submission.

Thanks again for reviewing.

Tim.

cdda-20171013.tgz (17K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: NEW games/cataclysm-dda

Brian Callahan-3
Hi Tim --

On 10/24/2017 8:44 PM, trondd wrote:
> Brian Callahan <[hidden email]> wrote:
>
>> The build moves on even if astyle fails. I'm not wedded to keeping it as
>> a BDEP. You can remove the astyle lines from cdda's Makefile if you like.
>>
> The build moves on if astyle is missing or too old.  It fails if there is
> an actual style issue.  I would have leaned towards excluding it based on
> reducing dependencies but since astyle doesn't bing in any more deps, I
> left it in for completeness.

In that case, this will have to be on hold until someone ok's the astyle
update. :)

>> Indeed you're right. It's a leftover from my trying something else. The
>> pre-configure routine can be reduced to:
>> pre-configure:
>>           sed -i -e 's,-Os,${CXXFLAGS},g' -e 's,-Werror,,g'
>> ${WRKSRC}/Makefile
>>
>> ~Brian
> Cleaned that up and I guess this is the final submission.

I think I'm good with this. ok for me (after the astyle update, of course).

~Brian

Reply | Threaded
Open this post in threaded view
|

Re: NEW games/cataclysm-dda

Stuart Henderson
In reply to this post by trondd-2
On 2017/10/24 20:44, trondd wrote:

> Brian Callahan <[hidden email]> wrote:
>
> >
> > The build moves on even if astyle fails. I'm not wedded to keeping it as
> > a BDEP. You can remove the astyle lines from cdda's Makefile if you like.
> >
>
> The build moves on if astyle is missing or too old.  It fails if there is
> an actual style issue.  I would have leaned towards excluding it based on
> reducing dependencies but since astyle doesn't bing in any more deps, I
> left it in for completeness.

The important thing as far as ports goes is whether having this installed
changes the build. If the build works and produces the same thing whether
astyle is installed or not, then it doesn't matter if the dep is listed or
not.

I must say it seems *really* weird to be running a source code formatting
tool during build.

> > Indeed you're right. It's a leftover from my trying something else. The
> > pre-configure routine can be reduced to:
> > pre-configure:
> >          sed -i -e 's,-Os,${CXXFLAGS},g' -e 's,-Werror,,g'
> > ${WRKSRC}/Makefile

I'm not a fan of sed with such a short string. Normally we just use patches.

Reply | Threaded
Open this post in threaded view
|

Re: NEW games/cataclysm-dda

trondd-2
Stuart Henderson <[hidden email]> wrote:

> On 2017/10/24 20:44, trondd wrote:
> > Brian Callahan <[hidden email]> wrote:
> >
> > >
> > > The build moves on even if astyle fails. I'm not wedded to keeping it as
> > > a BDEP. You can remove the astyle lines from cdda's Makefile if you like.
> > >
> >
> > The build moves on if astyle is missing or too old.  It fails if there is
> > an actual style issue.  I would have leaned towards excluding it based on
> > reducing dependencies but since astyle doesn't bing in any more deps, I
> > left it in for completeness.
>
> The important thing as far as ports goes is whether having this installed
> changes the build. If the build works and produces the same thing whether
> astyle is installed or not, then it doesn't matter if the dep is listed or
> not.
>
> I must say it seems *really* weird to be running a source code formatting
> tool during build.
>
I agree and have removed astyle and patched it out of their Makefile.

> > > Indeed you're right. It's a leftover from my trying something else. The
> > > pre-configure routine can be reduced to:
> > > pre-configure:
> > >          sed -i -e 's,-Os,${CXXFLAGS},g' -e 's,-Werror,,g'
> > > ${WRKSRC}/Makefile
>
> I'm not a fan of sed with such a short string. Normally we just use patches.

Replaced sed with patch changes.  Tweaked the methods of passing CXX* into
the build as well.  And it took me a while to figure out that specifying
MODLUA_VERSION removes the FLAVOR from the package name unless you set
MODLUA_SA as well.  Correct me if I missed something else with that.

Thanks.
Tim.


cdda-20171013.tgz (17K) Download Attachment