Pebaz / nimporter

Compile Nim Extensions for Python On Import!
MIT License
821 stars 33 forks source link

'-d:lto' flag breaks macOS #51

Closed p-i- closed 2 years ago

p-i- commented 3 years ago
    NIM_CLI_ARGS = [
        '--opt:speed',
        '--parallelBuild:0',
        '--gc:refc',
        '--threads:on',
        '--app:lib',
        '-d:release',
        '-d:strip',
        # '-d:lto',
        '-d:ssl',

I have to comment out the above line to achieve build on macOS.

It's here: https://github.com/Pebaz/nimporter/blob/master/nimporter.py#L130

The root seems to be https://github.com/nim-lang/Nim/pull/15614

This issue is blocking https://github.com/juancarlospaco/faster-than-requests/issues/156

Maybe needs something like:

if not user supplied(--os: macOS or macOSX):
    NIM_CLI_ARGS += '-d:lto'

But there's a bigger issue: These inbuilt flags can't be removed by the user's .cfg file.

This makes the library brittle.

Need some way to 'RESET' these internal defaults from the .cfg file, maybe? e.g. REMOVE='-d:lto'...

Pebaz commented 3 years ago

Full control over the command line switches can be done with a switches.py file.

However, the CLI args handling in Nimporter has always not been very ergonomic when trying to fiddle around with the Nim compiler.

I purposely designed this library to not require the user to enter a single customization option because it needs to be seamless for Python developers, who are very not used to dealing with compiler switches.

However, since users can use nim.cfg and switches.py, it might be time to remove the switches.py functionality and figure out a better option here. The bottom line is: in order to support out-of-the-box compilation with 0 configuration, default args must be chosen. However, overriding those args should be easier.

@SekouDiaoNlp Do you think you can look into either removing the -d:lto flag or refactoring Nimporter to more easily override these default arguments? Seems like nim.cfg is not the best choice since it can't override the NIM_CLI_ARGS. The switches.py file can, but it overlaps nim.cfg in functionality and Python developers won't use it anyway.

Pebaz commented 2 years ago

@SekouDiaoNlp sorry to hear that, glad your doing better! Yes that would be awesome to have a better solution to the flag override problem. It's possible that the switches.py functionality can be taken out entirely. In fact, that would be the first place I would look.

SekouDiaoNlp commented 2 years ago

@Pebaz ok, I will look into it during the next couple of days to come up with the most effective design. I think removing switches.py and consolidating all the config options in a single file + better handling of command line flags is the way to go.

SekouDiaoNlp commented 2 years ago

See discussion page : https://github.com/Pebaz/nimporter/discussions/54