Thoughts on Backporting the emit warning message to GEOS 3.6 and GEOS 3.5

Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Thoughts on Backporting the emit warning message to GEOS 3.6 and GEOS 3.5

Regina Obe
I'm only softly proposing this but want to get a feel if anyone has issues
before we do.

https://git.osgeo.org/gogs/geos/geos/pulls/14

I would like to back port this pull to GEOS 3.6 and GEOS 3.5 branches.

The reason being is

1) It's just a warning that shows when compiling code that uses the C++ API
so doesn't impact existing compiles.  No one has to change their compile
scripts.
But users using libgeos-dev for example from packages would be subjected to
the warning if they are using a project that uses the C++ API.

2) It gives people notice compiling projects using GEOS C++ API, that their
use may be in jeopardy in future and they might want to consider switching
to the C API or brace for breaking changes to the C++ API.


That warning will hopefully give folks enough time to switch to C-API so
that we feel comfortable doing some spring cleaning breaking
changes to the underlying C++ API in GEOS 3.8 or 4.0 with minimal effect on
existing users.


Thanks,
Regina

_______________________________________________
geos-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/geos-devel
Reply | Threaded
Open this post in threaded view
|

Re: Thoughts on Backporting the emit warning message to GEOS 3.6 and GEOS 3.5

Regina Obe
Slight correction I meant

"would be subjected to the warning if they are compiling source code that
uses the C++ API."

-----Original Message-----
From: Regina Obe [mailto:[hidden email]]
Sent: Friday, October 06, 2017 5:04 PM
To: 'GEOS Development List' <[hidden email]>
Subject: Thoughts on Backporting the emit warning message to GEOS 3.6 and
GEOS 3.5

I'm only softly proposing this but want to get a feel if anyone has issues
before we do.

https://git.osgeo.org/gogs/geos/geos/pulls/14

I would like to back port this pull to GEOS 3.6 and GEOS 3.5 branches.

The reason being is

1) It's just a warning that shows when compiling code that uses the C++ API
so doesn't impact existing compiles.  No one has to change their compile
scripts.
But users using libgeos-dev for example from packages would be subjected to
the warning if they are using a project that uses the C++ API.

2) It gives people notice compiling projects using GEOS C++ API, that their
use may be in jeopardy in future and they might want to consider switching
to the C API or brace for breaking changes to the C++ API.


That warning will hopefully give folks enough time to switch to C-API so
that we feel comfortable doing some spring cleaning breaking changes to the
underlying C++ API in GEOS 3.8 or 4.0 with minimal effect on existing users.


Thanks,
Regina

_______________________________________________
geos-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/geos-devel
Reply | Threaded
Open this post in threaded view
|

Re: Thoughts on Backporting the emit warning message to GEOS 3.6 and GEOS 3.5

Even Rouault-2
In reply to this post by Regina Obe

On vendredi 6 octobre 2017 17:03:58 CEST Regina Obe wrote:

> I'm only softly proposing this but want to get a feel if anyone has issues

> before we do.

>

> https://git.osgeo.org/gogs/geos/geos/pulls/14

>

> I would like to back port this pull to GEOS 3.6 and GEOS 3.5 branches.

>

> The reason being is

>

> 1) It's just a warning that shows when compiling code that uses the C++ API

> so doesn't impact existing compiles. No one has to change their compile

> scripts.

 

Not that I'd be impacted myself, but that is not true if they build with

-Werror, or expect that their build log remains clean if it was already.

 

(but my experience building GDAL with -Werror and most pedantic warnings turned on has demonstrated that for a lot of packages you have to include external headers with compiler mechanism to temporarily turn off warnings due to them)

 

Even

 

--

Spatialys - Geospatial professional services

http://www.spatialys.com


_______________________________________________
geos-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/geos-devel
Reply | Threaded
Open this post in threaded view
|

Re: Thoughts on Backporting the emit warning message to GEOS 3.6 and GEOS 3.5

Sandro Santilli-4
In reply to this post by Regina Obe
On Fri, Oct 06, 2017 at 05:03:58PM -0400, Regina Obe wrote:
> I'm only softly proposing this but want to get a feel if anyone has issues
> before we do.
>
> https://git.osgeo.org/gogs/geos/geos/pulls/14
>
> I would like to back port this pull to GEOS 3.6 and GEOS 3.5 branches.

I think only bugfixes should be backported.

--strk;
_______________________________________________
geos-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/geos-devel
Reply | Threaded
Open this post in threaded view
|

Re: Thoughts on Backporting the emit warning message to GEOS 3.6 and GEOS 3.5

Paul Norman
In reply to this post by Even Rouault-2
On 10/6/2017 2:13 PM, Even Rouault wrote:

> 1) It's just a warning that shows when compiling code that uses the C++ API

> so doesn't impact existing compiles. No one has to change their compile

> scripts.

 

Not that I'd be impacted myself, but that is not true if they build with

-Werror, or expect that their build log remains clean if it was already.


Yes - and for development and CI this is a good practice. That's why I think it's better to avoid adding this to old versions. It's not a versioning violating, but it will require some people to change scripts.

(but my experience building GDAL with -Werror and most pedantic warnings turned on has demonstrated that for a lot of packages you have to include external headers with compiler mechanism to temporarily turn off warnings due to them)

 


GEOS, GDAL, and other system libraries should be included with -isystem. This will stop warnings being generated for questionable syntax, etc. I don't think it will suppress warnings that *are* generated, such as in this case where there's an explicit preprocessor directive.

_______________________________________________
geos-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/geos-devel
Reply | Threaded
Open this post in threaded view
|

Re: Thoughts on Backporting the emit warning message to GEOS 3.6 and GEOS 3.5

Sandro Santilli-4
On Sat, Oct 07, 2017 at 03:30:26AM -0700, Paul Norman wrote:
> On 10/6/2017 2:13 PM, Even Rouault wrote:

> > -Werror, or expect that their build log remains clean if it was already.
>
> Yes - and for development and CI this is a good practice.

Pull requests adding -Werror to our CI bots are welcome
  .drone.yml     - PR to git.osgeo.org please
  .gitlab-ci.yml - PR to gitlab.com please
  .travis.yml    - PR to github.com please

I don't think Jenkins bots have in-repository configuration but
Regina might know better.

Ticketting this task would be also helpful.

--strk;
_______________________________________________
geos-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/geos-devel
Reply | Threaded
Open this post in threaded view
|

Re: Thoughts on Backporting the emit warning message to GEOS 3.6 and GEOS 3.5

Greg Troxel-2
In reply to this post by Sandro Santilli-4

Sandro Santilli <[hidden email]> writes:

> On Fri, Oct 06, 2017 at 05:03:58PM -0400, Regina Obe wrote:
>> I'm only softly proposing this but want to get a feel if anyone has issues
>> before we do.
>>
>> https://git.osgeo.org/gogs/geos/geos/pulls/14
>>
>> I would like to back port this pull to GEOS 3.6 and GEOS 3.5 branches.
>
> I think only bugfixes should be backported.

(I am not caught up on this whole thread) The point about -Werror is an
excellent one.  For that reason, I don't think backporting is a good
idea.  And I agree with strk about only bugfixes, more or less.

_______________________________________________
geos-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/geos-devel

signature.asc (167 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Thoughts on Backporting the emit warning message to GEOS 3.6 and GEOS 3.5

Regina Obe
Okay I'm fine with that.

Thanks all for input and Even's note about the -wError case.

-----Original Message-----
From: geos-devel [mailto:[hidden email]] On Behalf Of Greg Troxel
Sent: Saturday, October 07, 2017 10:25 AM
To: Sandro Santilli <[hidden email]>
Cc: GEOS Development List <[hidden email]>
Subject: Re: [geos-devel] Thoughts on Backporting the emit warning message to GEOS 3.6 and GEOS 3.5


Sandro Santilli <[hidden email]> writes:

> On Fri, Oct 06, 2017 at 05:03:58PM -0400, Regina Obe wrote:
>> I'm only softly proposing this but want to get a feel if anyone has
>> issues before we do.
>>
>> https://git.osgeo.org/gogs/geos/geos/pulls/14
>>
>> I would like to back port this pull to GEOS 3.6 and GEOS 3.5 branches.
>
> I think only bugfixes should be backported.

(I am not caught up on this whole thread) The point about -Werror is an excellent one.  For that reason, I don't think backporting is a good idea.  And I agree with strk about only bugfixes, more or less.

_______________________________________________
geos-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/geos-devel
Reply | Threaded
Open this post in threaded view
|

Re: Thoughts on Backporting the emit warning message to GEOS 3.6 and GEOS 3.5

Paul Norman
In reply to this post by Sandro Santilli-4
On 10/7/2017 4:17 AM, Sandro Santilli wrote:
> On Sat, Oct 07, 2017 at 03:30:26AM -0700, Paul Norman wrote:
>> On 10/6/2017 2:13 PM, Even Rouault wrote:
>>> -Werror, or expect that their build log remains clean if it was already.
>> Yes - and for development and CI this is a good practice.
> Pull requests adding -Werror to our CI bots are welcome

I was speaking of practices for users of GEOS. I'm not developing any
libraries currently, so I don't know what best practices are there.
_______________________________________________
geos-devel mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/geos-devel