Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'-d:lto' flag breaks macOS #51

Closed
p-i- opened this issue Jun 23, 2021 · 4 comments · Fixed by #55
Closed

'-d:lto' flag breaks macOS #51

p-i- opened this issue Jun 23, 2021 · 4 comments · Fixed by #55
Assignees
Labels
enhancement New feature or request

Comments

@p-i-
Copy link

p-i- commented Jun 23, 2021

    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 nim-lang/Nim#15614

This issue is blocking juancarlospaco/faster-than-requests#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
Copy link
Owner

Pebaz commented Jun 23, 2021

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
Copy link
Owner

Pebaz commented Oct 31, 2021

@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
Copy link
Collaborator

@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 SekouDiaoNlp self-assigned this Oct 31, 2021
@SekouDiaoNlp SekouDiaoNlp added the enhancement New feature or request label Oct 31, 2021
@SekouDiaoNlp
Copy link
Collaborator

See discussion page : #54

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants