Skip to content

Conversation

@hahnjo
Copy link
Member

@hahnjo hahnjo commented Jan 14, 2025

All of them require (optional) dependencies for very little use.

FYI @andresailer

@hahnjo hahnjo added in:Build System clean build Ask CI to do non-incremental build on PR labels Jan 14, 2025
@hahnjo hahnjo requested a review from dpiparo January 14, 2025 10:58
@hahnjo hahnjo self-assigned this Jan 14, 2025
@hahnjo hahnjo requested a review from bellenot as a code owner January 14, 2025 10:59
@cern-sft-spi
Copy link

Thanks for the heads up.
We still had mysql=ON.

It is enough to turn the options OFF, right?

@hahnjo
Copy link
Member Author

hahnjo commented Jan 14, 2025

If nobody uses it, yes. The deprecation message will print if the option is set to ON. Eventually you will not want to set the options at all to prepare for removal.

@jblomer
Copy link
Contributor

jblomer commented Jan 14, 2025

I'm in favor of deprecating mysql, odbc, pgsql. But not sqlite. It is used, e.g., by cvmfs to use RDataFrame on sqlite data.

@github-actions
Copy link

github-actions bot commented Jan 14, 2025

Test Results

    18 files      18 suites   4d 7h 48m 2s ⏱️
 2 694 tests  2 692 ✅ 0 💤 2 ❌
46 762 runs  46 760 ✅ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit 077b6d1.

♻️ This comment has been updated with latest results.

All of them require (optional) dependencies for very little use.
@hahnjo hahnjo changed the title [cmake] Deprecate mysql, odbc, pgsql, sqlite build options [cmake] Deprecate mysql, odbc, pgsql build options Jan 14, 2025
@hahnjo
Copy link
Member Author

hahnjo commented Jan 14, 2025

Oh yes, my bad: I didn't realize sqlite was also used in places other than sql. I reverted the change to the sqlite option, we can decide independently if we want to deprecate sql/sqlite and the various TSQL* classes spread between io/sql and net/net. For the moment, I'm focusing on the "easy" deprecations...

@bellenot
Copy link
Member

AFAIK, the odbc one is mostly for Windows and comes for free (i.e. It is available when installing Visual Studio). So we could maybe keep it? Or is it too much of a overhead?

@hahnjo
Copy link
Member Author

hahnjo commented Jan 17, 2025

AFAIK, the odbc one is mostly for Windows and comes for free (i.e. It is available when installing Visual Studio). So we could maybe keep it? Or is it too much of a overhead?

I think the main question for me is whether it is used. Do we know of anybody relying on it? Eventually I'd personally like to see all SQL-related classes deprecated and removed from ROOT because I don't think we should encourage their use.

@bellenot
Copy link
Member

AFAIK, the odbc one is mostly for Windows and comes for free (i.e. It is available when installing Visual Studio). So we could maybe keep it? Or is it too much of a overhead?

I think the main question for me is whether it is used. Do we know of anybody relying on it? Eventually I'd personally like to see all SQL-related classes deprecated and removed from ROOT because I don't think we should encourage their use.

OK fair enough, let's deprecate it and we'll see

Copy link
Contributor

@guitargeek guitargeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@hahnjo hahnjo merged commit 8ded485 into root-project:master Jan 21, 2025
18 of 21 checks passed
@hahnjo hahnjo deleted the deprecate-sql branch January 21, 2025 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clean build Ask CI to do non-incremental build on PR in:Build System

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants