Skip to content

Conversation

cfis
Copy link
Contributor

@cfis cfis commented Feb 14, 2025

is invalid with MSVC. It it is set correctly here - https://github.com/duckdb/duckdb/blob/main/tools/pythonpkg/setup.py#L162 and L165, but then reset again at https://github.com/duckdb/duckdb/blob/main/tools/pythonpkg/setup.py#L176 but incorrectly for all compilers.

@Mytherin Mytherin merged commit 2951ea0 into duckdb:main Feb 14, 2025
25 checks passed
@Mytherin
Copy link
Collaborator

Thanks!

@Tishj
Copy link
Contributor

Tishj commented Feb 17, 2025

The title is "fix", if this was called "remove" I would have looked at it, because this isn't fixing
I am glad I remember this was recently changed because this caused my build to fail on macOS

I'll try to find a solution that fixes this for all platforms

@Tishj
Copy link
Contributor

Tishj commented Feb 17, 2025

This doesn't sound like it's possible to reliably detect reading through some related work on the topic of detecting MSVC
Notably the encouraging comment here
https://github.com/jodhus/pyebur128/blob/2fa5defe08af71d4c2ff85634f1498daea1fcaf4/setup.py#L19

@Mytherin
Copy link
Collaborator

Can we just use if os.name == 'nt': which we use elsewhere in the script?

@Tishj
Copy link
Contributor

Tishj commented Feb 17, 2025

Do we care about differentiating between MinGW and MSVC? Because that check does not differentiate

@Mytherin
Copy link
Collaborator

I don't think we do

@cfis
Copy link
Contributor Author

cfis commented Feb 17, 2025

Sorry, didn't mean to break a build! Why wouldn't the c++11 flag be set a few line above in the code though?

You likely do want to distinguish between msvc and mingw because the compiler flags are different. It is easy to tell from CMAKE, _WIN32 will be set as well as GCC (or _MSC_VER will or won't be set). Maybe that could be passed in to the python code?

Mytherin added a commit that referenced this pull request Feb 17, 2025
…n the compiler (MSVC or not) (#16267)

This PR addresses #16237

My local builds started failing because `-std=c++11` was missing, this
should fix the issue for both flavors of compiler.
Please verify that this doesn't break the MSVC environment @cfis
Antonov548 added a commit to Antonov548/duckdb-r that referenced this pull request Feb 27, 2025
Issue duckdb/duckdb#8265: AsOf Nested Loop (duckdb/duckdb#16218)
Fix -std=c++11 (duckdb/duckdb#16237)
Fix Python 3 executable name on Windows (duckdb/duckdb#16236)
Use _win32 with MSVC (duckdb/duckdb#16235)
Do In-Filter pushdown in PyArrow (duckdb/duckdb#16224)
krlmlr pushed a commit to duckdb/duckdb-r that referenced this pull request Mar 3, 2025
Issue duckdb/duckdb#8265: AsOf Nested Loop (duckdb/duckdb#16218)
Fix -std=c++11 (duckdb/duckdb#16237)
Fix Python 3 executable name on Windows (duckdb/duckdb#16236)
Use _win32 with MSVC (duckdb/duckdb#16235)
Do In-Filter pushdown in PyArrow (duckdb/duckdb#16224)
Antonov548 added a commit to Antonov548/duckdb-r that referenced this pull request Mar 4, 2025
Issue duckdb/duckdb#8265: AsOf Nested Loop (duckdb/duckdb#16218)
Fix -std=c++11 (duckdb/duckdb#16237)
Fix Python 3 executable name on Windows (duckdb/duckdb#16236)
Use _win32 with MSVC (duckdb/duckdb#16235)
Do In-Filter pushdown in PyArrow (duckdb/duckdb#16224)
krlmlr pushed a commit to duckdb/duckdb-r that referenced this pull request Mar 5, 2025
Issue duckdb/duckdb#8265: AsOf Nested Loop (duckdb/duckdb#16218)
Fix -std=c++11 (duckdb/duckdb#16237)
Fix Python 3 executable name on Windows (duckdb/duckdb#16236)
Use _win32 with MSVC (duckdb/duckdb#16235)
Do In-Filter pushdown in PyArrow (duckdb/duckdb#16224)
krlmlr pushed a commit to duckdb/duckdb-r that referenced this pull request Mar 5, 2025
Issue duckdb/duckdb#8265: AsOf Nested Loop (duckdb/duckdb#16218)
Fix -std=c++11 (duckdb/duckdb#16237)
Fix Python 3 executable name on Windows (duckdb/duckdb#16236)
Use _win32 with MSVC (duckdb/duckdb#16235)
Do In-Filter pushdown in PyArrow (duckdb/duckdb#16224)
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request May 15, 2025
Issue duckdb/duckdb#8265: AsOf Nested Loop (duckdb/duckdb#16218)
Fix -std=c++11 (duckdb/duckdb#16237)
Fix Python 3 executable name on Windows (duckdb/duckdb#16236)
Use _win32 with MSVC (duckdb/duckdb#16235)
Do In-Filter pushdown in PyArrow (duckdb/duckdb#16224)
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request May 15, 2025
Issue duckdb/duckdb#8265: AsOf Nested Loop (duckdb/duckdb#16218)
Fix -std=c++11 (duckdb/duckdb#16237)
Fix Python 3 executable name on Windows (duckdb/duckdb#16236)
Use _win32 with MSVC (duckdb/duckdb#16235)
Do In-Filter pushdown in PyArrow (duckdb/duckdb#16224)
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request May 17, 2025
Issue duckdb/duckdb#8265: AsOf Nested Loop (duckdb/duckdb#16218)
Fix -std=c++11 (duckdb/duckdb#16237)
Fix Python 3 executable name on Windows (duckdb/duckdb#16236)
Use _win32 with MSVC (duckdb/duckdb#16235)
Do In-Filter pushdown in PyArrow (duckdb/duckdb#16224)
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request May 18, 2025
Issue duckdb/duckdb#8265: AsOf Nested Loop (duckdb/duckdb#16218)
Fix -std=c++11 (duckdb/duckdb#16237)
Fix Python 3 executable name on Windows (duckdb/duckdb#16236)
Use _win32 with MSVC (duckdb/duckdb#16235)
Do In-Filter pushdown in PyArrow (duckdb/duckdb#16224)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants