Skip to content

Conversation

althonos
Copy link
Member

@althonos althonos commented Feb 24, 2018

Rationale

The behaviour of build_ext is the following: when generating compilation
artifacts (such as .o files), it will put them in a directory defined by the
build_temp argument, and within that directory create a directory named after
the project name in setup.py, and them dump its temporary files there.

The only way to do that with build_rust was through editing the
CARGO_TARGET_DIR environment variable, but this is not practical and also does
not comply well with the different available configuration files (setup.cfg,
user distutils.cfg and the like).

Implementation

  • if the user gives the --build-temp argument to the CLI, or sets build_temp
    within the [build], [build_ext] or [build_rust] sections of any
    configuration file, then cargo will use that directory as a target directory.
  • if no directory is set, then cargo will use the default build_temp (which is
    build/temp.<platform>-<version>).
  • if CARGO_TARGET_DIR is set, then it will overwrite any python-defined setting
    and build to that directory directly.

I also made build_rust inherit from build_ext the following options: inplace
and debug.

Perks

  • if a library has more than one extension modules with Cargo.toml files in
    different subdirectories, cargo would create a local target directory for each
    library, and thus rebuild several times the same cargo modules (for instance,
    PyO3 !)
  • removes the need for a clean_rust command, since python setup.py clean
    removes the build_temp directory
  • fixes Patched commands ignore setup.cfg defined options #27 😉
  • fixes the following error: when CARGO_TARGET_DIR is set, and multiple extensions are built for a project, the wrong library can be copied (since build_rust did not check for the library name beforehand)

Drawbacks

  • not really backwards compatible

Martin Larralde added 4 commits February 24, 2018 14:00
* `build_rust` will try to inherit the following settings from `build_ext`:
     - `inplace`
     - `build_temp`
     - `debug`
* `build_rust` will compile extensions into the `build_temp` directory,
  which defaults to `build/temp.<platform>-<version>
@adrienball
Copy link

adrienball commented Feb 27, 2018

+1

I am running into troubles with a rust extension generating multiple .dylib files in the target dir. The following piece of code in setuptools_rust picks the first binary matching the wildcard, which varies across platforms:

if sys.platform == "win32":
    wildcard_so = "*.dll"
elif sys.platform == "darwin":
    wildcard_so = "*.dylib"
else:
    wildcard_so = "*.so"

try:
    dylib_paths.append(
        (ext.name, glob.glob(
            os.path.join(target_dir, wildcard_so))[0]))
except IndexError:
    raise DistutilsExecError(
        "rust build failed; unable to find any %s in %s" %
        (wildcard_so, target_dir))

@fafhrd91
Copy link
Collaborator

awesome! thanks

@fafhrd91 fafhrd91 merged commit 5e7500d into PyO3:master Feb 27, 2018
@fafhrd91
Copy link
Collaborator

@althonos do you want me to make new release?

@althonos
Copy link
Member Author

@fafhrd91: yes, that would be nice of you, I have a project that would likely benefit from it !

I'm also working on an automatic Cargo.toml generation for extensions based on source files and package metadata, would you be interested in a PR to integrate it or should I rather work on it as an external package ?

@fafhrd91
Copy link
Collaborator

Auto generating cargo toml seems interesting idea

@fafhrd91
Copy link
Collaborator

new version is released

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.

Patched commands ignore setup.cfg defined options

3 participants