Hello all,
I posted this patch some time ago, but I just noticed today that it not
applied. I thought it had been, but maybe it was backed out for some
reason.
I post again. It removes "struct Tape_Info" and related functions. It
also removes I_malloc(), I_realloc() and I_free() in favor of G_*().
Any objections to me applying or did someone find a (legacy) use for the
tape functions?
--
Brad Douglas <rez@touchofmadness.com>
imagery.patch (9.42 KB)
Brad Douglas wrote:
I posted this patch some time ago, but I just noticed today that it not
applied. I thought it had been, but maybe it was backed out for some
reason.I post again. It removes "struct Tape_Info" and related functions. It
also removes I_malloc(), I_realloc() and I_free() in favor of G_*().Any objections to me applying or did someone find a (legacy) use for the
tape functions?
None here.
Also, as you have already identified the I_malloc/I_realloc calls, it
would be nice to remove the type casts. As G_malloc/G_realloc return
void*, the casts are unnecessary.
--
Glynn Clements <glynn@gclements.plus.com>
I posted this patch some time ago, but I just noticed today that it
not applied. I thought it had been, but maybe it was backed out for
some reason.
It's easy to view the CVS history on the web to keep abreast of changes:
http://freegis.org/cgi-bin/viewcvs.cgi/grass6/include/imagedefs.h
Hamish
Glynn Clements wrote:
Brad Douglas wrote:
I posted this patch some time ago, but I just noticed today that it not
applied. I thought it had been, but maybe it was backed out for some
reason.I post again. It removes "struct Tape_Info" and related functions. It
also removes I_malloc(), I_realloc() and I_free() in favor of G_*().Any objections to me applying or did someone find a (legacy) use for the
tape functions?None here.
Also, as you have already identified the I_malloc/I_realloc calls, it
would be nice to remove the type casts. As G_malloc/G_realloc return
void*, the casts are unnecessary.
Brad, you probably need CVS account?
I am also in favour of code cleaning but is it safe to remove functions from GRASS libraries during the 6.x line? Cannot that break backward compatibility? GDAL for example is using libgrass_I.
I thought that we want to maintain binary compatibility for all 6.x releases, is it right? If yes, what are the rules we have to follow?
I thought that at least:
- existing functions must not be changed, removed or renamed
- structures must not be changed
- constants and enumerations must not be changed
Radim
Can we come to some consensus before this drives me crazy?
On Tue, 2005-05-10 at 10:12 +0200, Radim Blazek wrote:
Brad Douglas wrote:
>>>>I post again. It removes "struct Tape_Info" and related functions. It
>>>>also removes I_malloc(), I_realloc() and I_free() in favor of G_*().
>>>>
>>>>Any objections to me applying or did someone find a (legacy) use for the
>>>>tape functions?
>>>
>>>None here.
>>>
>>>Also, as you have already identified the I_malloc/I_realloc calls, it
>>>would be nice to remove the type casts. As G_malloc/G_realloc return
>>>void*, the casts are unnecessary.>>I am also in favour of code cleaning but is it safe to remove functions
>>from GRASS libraries during the 6.x line? Cannot that break backward
>>compatibility? GDAL for example is using libgrass_I.
>
>
> Hmm. I've had some discussion off-list with Glynn about lib/imagery
> changes. I asked Glynn if wee were in a full development cycle and he
> indicated that we were. Do we have a loose hierarchy as to who oversees
> what?What does it mean 'a full development cycle'? I don't believe we are in
such state after 6.0.0 release.'Loose hierarchy' sounds too strongly.
> In this particular case, I maybe one compatibility issue since I_malloc
> () was the only function actually in use (IIRC). None of the tape_info
> structures/functions are in use anywhere (looks like support was dropped
> in 5.x -> 6.x, but function definitions not removed).I am almost sure that these changes cannot cause problems but we have to
follow some strategy. You cannot say 'non of the structures/functions
are used' because many users have developed their custom modules and we
cannot break their installations within 6.x.>>I thought that we want to maintain binary compatibility for all 6.x
>>releases, is it right? If yes, what are the rules we have to follow?
>>I thought that at least:
>>- existing functions must not be changed, removed or renamed
>>- structures must not be changed
>>- constants and enumerations must not be changed
>
> No problem. I'm still quite new to the "flow" of GRASS development and
> I seem to be getting different direction from different people.The rules for compatibility maintenance probably were not explicitly
written. The only person here who can define them is Glynn.In any case, such things should be discussed in the mailing list.
Radim
--
Brad Douglas <rez@touchofmadness.com>
> >>>>I post again. It removes "struct Tape_Info" and related
> >>>> functions. It also removes I_malloc(), I_realloc() and
> >>>> I_free() in favor of G_*().
> >>>> Any objections to me applying or did someone find a (legacy)
> >>>> use for the tape functions?
> >>>
> >>>None here.
> >>>
> >>>Also, as you have already identified the I_malloc/I_realloc
> >>>calls, it would be nice to remove the type casts. As
> >>>G_malloc/G_realloc return void*, the casts are unnecessary.
Remove the relevant I_*() functions from the operational code but leave
the library functions in place, but never called.
If you are sure they are now unused, perhaps surround them with
#define LEGACY_CODE
#ifdef LEGACY_CODE
int I_malloc()
#endif
so when we are in loose development mode it is clear what stuff is
scheduled to be ripped out and effects can be quickly enacted without
too much effort or risk. Adding a "depreciated" note to the Doxygen
comments is probably in order as well. Perhaps as a G_warning() as well
within the function so it is obvious if we missed any calls to it.
> > No problem. I'm still quite new to the "flow" of GRASS
> > development and I seem to be getting different direction from
> > different people.
yeah well... we've fallen off the end of the roadmap,
http://grass.ibiblio.org/devel/roadmap.php
so I guess it needs some updating.
to add another sample point re. moving forward in all directions:
executive summary- I think the "will it break a script or plugin"
test is a good one for current development.
[note to reader: long winded, no technical insights, feel free to move
on with your day without feeling you've missed anything at this point.]
My feeling is that the 6.0 series is very young and will be around for
a while so we need to continue to let it stabilize and knock out more
bugs. e.g. if a new edition to the GRASS book is written for GRASS 6, it
would be good for the authors, readers, and students if the interface
was somewhat stable and the examples work as advertised, etc. Along
these lines the 6.0-beta series really demonstrated how well we could
concentrate effort onto fixing bugs when we were focused on doing so.
So at least try and make things backwards compatible back as far as the
6.0.0 release. I guess the main idea is that a script should be able to
be written for "GRASS 6" and work with any given version of "GRASS 6".
The code behind binary plugins shouldn't have to be changed between
minor releases (6.0.0, 6.0.1, ..., 6.0.26, etc.).
I think we should certainly keep the data formats interchangeable during
anything we call "6" but I'm not too concerned if an internal GRASS
6.0.0 library isn't 100% compatible with say version 6.2.0 (but others
might be; I'm no API guru). It is a courtesy to others like QGIS/GDAL/R
plugins to not break things with every minor point release, although
that is probably not often enough to get too ugly (I mean between 6.0.x,
6.2.x, etc.). But again, just my US$0.0146.
The trick is to find the sweet spot where you don't stall development so
much so that updates get forgotten and abandoned, motivation moves
somewhere else, etc.. Maybe a Wiki or CVS page of things TODO once
development mode is wide open? We would have to define what version (if
not when by date) that will be. In the past these TODO lists have been
scattered and not well advertised, ie ignored. Perhaps it would be
useful to have a TODO directory of sorts pointing to the different docs
& readmes in the wiki, with links to where the files are in the CVS
tree. I prefer the clear history CVS provides to an open wiki for the
actual files for two reasons: a) when a release is made it becomes
frozen in time & you can cross reference the code, and b) otherwise it
might as well be a unofficial wishlist-tracker.
You can fundamentally fix more bugs by condensing and cleaning the code
base, but you do risk breaking lots of things in the short term.
Hamish
Hamish wrote:
So at least try and make things backwards compatible back as far as the
6.0.0 release. I guess the main idea is that a script should be able to
be written for "GRASS 6" and work with any given version of "GRASS 6".
The code behind binary plugins shouldn't have to be changed between
minor releases (6.0.0, 6.0.1, ..., 6.0.26, etc.).
Why only between minor versions? I think that binary compatibility must be between all 6.x versions. I cannot imagine that GDAL 1.2.10 will depend on GRASS 6.0.3 but not with GRASS 6.2.0! We have already many new things in GRASS 6.1 and GRASS 6.2 will be release in few monts. It must not break existing GDAL, QGIS and whatever.
I think we should certainly keep the data formats interchangeable during
anything we call "6" but I'm not too concerned if an internal GRASS
6.0.0 library isn't 100% compatible with say version 6.2.0 (but others
might be; I'm no API guru).
Yes I am concerned, GRASS is no more isolated island, other applications like GDAL, QGIS and custom applications are using GRASS libraries.
It is a courtesy to others like QGIS/GDAL/R
plugins to not break things with every minor point release,
Minor versions are 6.x, right? 6 is major version 6.0 is minor version and 6.0.0 is release.
Radim
although
that is probably not often enough to get too ugly (I mean between 6.0.x,
6.2.x, etc.). But again, just my US$0.0146.
On Wed, May 11, 2005 5:03, Hamish said:
Maybe a Wiki or CVS page of things TODO once
development mode is wide open? We would have to define what version (if
not when by date) that will be. In the past these TODO lists have been
scattered and not well advertised, ie ignored. Perhaps it would be
useful to have a TODO directory of sorts pointing to the different docs
& readmes in the wiki, with links to where the files are in the CVS
tree. I prefer the clear history CVS provides to an open wiki for the
actual files for two reasons: a) when a release is made it becomes
frozen in time & you can cross reference the code, and b) otherwise it
might as well be a unofficial wishlist-tracker.
A beginning of a general Wiki page is here:
http://grass.gdf-hannover.de/twiki/bin/view/GRASS/GrassToDoList
but a specific page for developers is needed.
Moritz