[gdal-dev] [Proposal] Move optional raw datasets to separate directory

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

[gdal-dev] [Proposal] Move optional raw datasets to separate directory

tbonfort
Following #1250 [1] that allows deactivating drivers at configure time, in the interest of cutting down on compile time, generated binary size and surface of attack in case of flaws, I would like to propose to be able to split the frmts/raw directory in 2.

raw/ would only contain rawdataset.cpp|h and would be a mandatory driver (it is required by the vrt system)

rawmisc/ (or another name to be proposed) would contain all the other files, and could be optionally disabled at configure time as in #1250

I'll propose a pull request for this, but prefer waiting on concensus here before starting the task.

Best regards,
Thomas



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

Re: [Proposal] Move optional raw datasets to separate directory

Even Rouault-2
Thomas,

> Following #1250 [1] that allows deactivating drivers at configure time,
> in the interest of cutting down on compile time, generated binary size and
> surface of attack in case of flaws, I would like to propose to be able to
> split the frmts/raw directory in 2.

Is the split in 2 directories for aesthetic reasons, or because it makes the
conditional compiling logic easier ?

>
> raw/ would only contain rawdataset.cpp|h and would be a mandatory driver
> (it is required by the vrt system)

Technically/pedanticaly this is not a driver (there's no "RAW" driver by
itself), but a base class extended by other drivers.

>
> rawmisc/ (or another name to be proposed) would contain all the other
> files, and could be optionally disabled at configure time as in #1250

"rawdrivers" maybe ?

Even

--
Spatialys - Geospatial professional services
http://www.spatialys.com
_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Move optional raw datasets to separate directory

Even Rouault-2
(Re-adding the list)

> > Is the split in 2 directories for aesthetic reasons, or because it makes
> > the
> > conditional compiling logic easier ?
>
> The latter. I'd like to avoid modifying the cpp files themselves to add
> ifdefs to them.

OK, I'm not clear why the split would change something in that respect. Or
perhaps I misunderstood your proposals: would be the enabling/disabling
compilation of raw drivers be for all raw drivers at a time, or per-driver ? I
initially assumed the later, but guess this must be the former, given what you
mention above. Not that I particularly care about either way, but just for the
sake of my understanding.

>
> > > raw/ would only contain rawdataset.cpp|h and would be a mandatory driver
> > > (it is required by the vrt system)
> So it could go to gcore ? That would have less overhead all-in-all.

Indeed I was considering this as an option. There are a number of GNUmakefile/
makefile.vc with -I../raw in them that could be cleaned up a bit then.

--
Spatialys - Geospatial professional services
http://www.spatialys.com
_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Proposal] Move optional raw datasets to separate directory

tbonfort


Le ven. 1 févr. 2019 à 18:10, Even Rouault <[hidden email]> a écrit :
(Re-adding the list)

> > Is the split in 2 directories for aesthetic reasons, or because it makes
> > the
> > conditional compiling logic easier ?
>
> The latter. I'd like to avoid modifying the cpp files themselves to add
> ifdefs to them.

OK, I'm not clear why the split would change something in that respect. Or
perhaps I misunderstood your proposals: would be the enabling/disabling
compilation of raw drivers be for all raw drivers at a time, or per-driver ? I
initially assumed the later, but guess this must be the former, given what you
mention above. Not that I particularly care about either way, but just for the
sake of my understanding.
yes correct, I was thinking of all raw drivers or none.

>
> > > raw/ would only contain rawdataset.cpp|h and would be a mandatory driver
> > > (it is required by the vrt system)
> So it could go to gcore ? That would have less overhead all-in-all.

Indeed I was considering this as an option. There are a number of GNUmakefile/
makefile.vc with -I../raw in them that could be cleaned up a bit then.
if that's ok with you I can incorporate that in #1250.


--
Spatialys - Geospatial professional services
http://www.spatialys.com

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

Re: [Proposal] Move optional raw datasets to separate directory

Andrew C Aitchison-2
In reply to this post by Even Rouault-2
On Fri, 1 Feb 2019, Even Rouault wrote:

> (Re-adding the list)
>
>>> Is the split in 2 directories for aesthetic reasons, or because it makes
>>> the
>>> conditional compiling logic easier ?
>>
>> The latter. I'd like to avoid modifying the cpp files themselves to add
>> ifdefs to them.

Most of the drivers I have looked at have a suitable ifdef,
so I checked and was surprised to see that most do not
(a rough grep suggests that 28 frmts directories have such an ifdef and 93
do not).

The pull-request talks about
./configure
  ... \
  disable-driver-nnnn \
  ...

When using this proposal to generate minimal binaries, would
it not be more natural to name the drivers that you wish to *enable*
and have the configure system add whatever dependencies are required ?
When a new driver ws added, I would like to assume that my minimal
build would ignore it, without having to alter my build command.

I'm not sure about making "virt" one of the few required drivers;
it would be hard to determine which virt files would be supported
since a single target could use any of the other drivers.

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

Re: [Proposal] Move optional raw datasets to separate directory

tbonfort
hi,

Le ven. 1 févr. 2019 à 20:39, Andrew C Aitchison <[hidden email]> a écrit :
On Fri, 1 Feb 2019, Even Rouault wrote:

> (Re-adding the list)
>
>>> Is the split in 2 directories for aesthetic reasons, or because it makes
>>> the
>>> conditional compiling logic easier ?
>>
>> The latter. I'd like to avoid modifying the cpp files themselves to add
>> ifdefs to them.

Most of the drivers I have looked at have a suitable ifdef,
so I checked and was surprised to see that most do not
(a rough grep suggests that 28 frmts directories have such an ifdef and 93
do not).

The pull-request talks about
./configure
        ... \
        disable-driver-nnnn \
        ...

When using this proposal to generate minimal binaries, would
it not be more natural to name the drivers that you wish to *enable*
and have the configure system add whatever dependencies are required ?
When a new driver ws added, I would like to assume that my minimal
build would ignore it, without having to alter my build command.
I'm not sure you were looking at the latest version of the pull request. in your case, you'd use
./configure --disable-all-drivers --enable-driver-nnnn
and be assured that only the nnnn driver and the mandatory ones are included.

I'm not sure about making "virt" one of the few required drivers;
it would be hard to determine which virt files would be supported
since a single target could use any of the other drivers.
that is already the case actually, although it currently only concerns drivers that would have been added by a --with-xxxx.
I do believe it is useful to be able to use vrts along with the subset of drivers you have chosen to enable.

Note that the target of this PR is not to generate a generic gdal build, but rather to generate a minimal build that contains only the formats you know will be needed, typically when building a custom docker image.

best regards,
 thomas


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

Re: [Proposal] Move optional raw datasets to separate directory

tbonfort
for the sake of clarity, I think I should rename the --disable-all-drivers flag to --disable-all-optional-drivers ...

thomas

Le ven. 1 févr. 2019 à 20:58, thomas bonfort <[hidden email]> a écrit :
hi,

Le ven. 1 févr. 2019 à 20:39, Andrew C Aitchison <[hidden email]> a écrit :
On Fri, 1 Feb 2019, Even Rouault wrote:

> (Re-adding the list)
>
>>> Is the split in 2 directories for aesthetic reasons, or because it makes
>>> the
>>> conditional compiling logic easier ?
>>
>> The latter. I'd like to avoid modifying the cpp files themselves to add
>> ifdefs to them.

Most of the drivers I have looked at have a suitable ifdef,
so I checked and was surprised to see that most do not
(a rough grep suggests that 28 frmts directories have such an ifdef and 93
do not).

The pull-request talks about
./configure
        ... \
        disable-driver-nnnn \
        ...

When using this proposal to generate minimal binaries, would
it not be more natural to name the drivers that you wish to *enable*
and have the configure system add whatever dependencies are required ?
When a new driver ws added, I would like to assume that my minimal
build would ignore it, without having to alter my build command.
I'm not sure you were looking at the latest version of the pull request. in your case, you'd use
./configure --disable-all-drivers --enable-driver-nnnn
and be assured that only the nnnn driver and the mandatory ones are included.

I'm not sure about making "virt" one of the few required drivers;
it would be hard to determine which virt files would be supported
since a single target could use any of the other drivers.
that is already the case actually, although it currently only concerns drivers that would have been added by a --with-xxxx.
I do believe it is useful to be able to use vrts along with the subset of drivers you have chosen to enable.

Note that the target of this PR is not to generate a generic gdal build, but rather to generate a minimal build that contains only the formats you know will be needed, typically when building a custom docker image.

best regards,
 thomas


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