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

Problems changing PATH in windows #765

Closed
dvarrazzo opened this issue Jul 17, 2021 · 17 comments · Fixed by #767
Closed

Problems changing PATH in windows #765

dvarrazzo opened this issue Jul 17, 2021 · 17 comments · Fixed by #767

Comments

@dvarrazzo
Copy link
Contributor

Hello, I am trying to use cibuildwheel to build psycopg >= 3 packages for windows.

The package requires a dll, libpq.dll to build. The location of the dll is given by a program called pg_config which, in the build environment, is installed in a directory not under path.

I am using this configuration in order to try the build, containing:

          CIBW_ENVIRONMENT_WINDOWS: >-
            PATH='C:\Program Files\PostgreSQL\13\bin;C:\Program Files\PostgreSQL\13\lib;$PATH'

However build fails in a way that suggests that the path is not prepended, but rather replaced:

Run pypa/cibuildwheel@v1.12.0

     _ _       _ _   _       _           _
 ___|_| |_ _ _|_| |_| |_ _ _| |_ ___ ___| |
|  _| | . | | | | | . | | | |   | -_| -_| |
|___|_|___|___|_|_|___|_____|_|_|___|___|_|

cibuildwheel version 1.12.0

Build options:
  platform: 'windows'
  [...]
  environment: ParsedEnvironment(['PSYCOPG_IMPL=binary', "PSYCOPG_TEST_DSN='host=172.17.0.1 user=postgres'", "PATH='C:\\Program Files\\PostgreSQL\\13\\bin;C:\\Program Files\\PostgreSQL\\13\\lib;$PATH'"])
  [...]

Here we go!

Running before_all...
  
  + powershell.exe tools\build\wheel_windows_before_all.ps1
  'powershell.exe' is not recognized as an internal or external command,
  operable program or batch file.
Error: Command powershell.exe tools\build\wheel_windows_before_all.ps1 failed with code 1. None

Is there anything wrong in what I am doing?

@joerick
Copy link
Contributor

joerick commented Jul 18, 2021

The environment variables are expanded with bash syntax. In bash, single-quotes don't expand variables, so you probably want to remove the single-quote from that string. double-quotes, or no quotes should work for that option.

dvarrazzo added a commit to psycopg/psycopg that referenced this issue Jul 18, 2021
A test to verify if pypa/cibuildwheel#765 can
be worked around this way.
@dvarrazzo
Copy link
Contributor Author

Hi @joerick

That makes sense. I was confused because I thought that the single quotes were an escape mechanism to pass data from a single yaml string to whatever parses the environ.

I've made a few tests, and I've seen that yes, the double quotes allow the $PATH expansion, but within double quotes backslashes need to be escapd. Using something like:

          CIBW_ENVIRONMENT_WINDOWS: >-
            PATH="C:\\Program Files\\PostgreSQL\\13\\bin;$PATH"

works as expected.

I will send a MR to add a similar example to the docs.

@dvarrazzo
Copy link
Contributor Author

There is an extra layer of complexity because quoted strings in yaml require backslash escaping, but the above is not a quoted yaml string, altough it contains a yaml string in quotes...

dvarrazzo added a commit to dvarrazzo/cibuildwheel that referenced this issue Jul 18, 2021
On Windows you often find directories containing spaces. The relation
between quotes and escaping is not intuitive, see pypa#765.
@joerick
Copy link
Contributor

joerick commented Jul 19, 2021

There is an extra layer of complexity because quoted strings in yaml require backslash escaping, but the above is not a quoted yaml string, altough it contains a yaml string in quotes...

Eeek. Yes, good point. I think most users don't need quotes at all, but in your example you have spaces so it's required. By the way, I think it's the bash parsing that's collapsing the double-backslash to a single one, not YAML.

bash-3.2$ echo "C:\\Program Files\\PostgreSQL\\13\\bin"
C:\Program Files\PostgreSQL\13\bin

@joerick joerick linked a pull request Jul 19, 2021 that will close this issue
@dvarrazzo
Copy link
Contributor Author

What I meant is that examples such as: CIBW_ENVIRONMENT: "BUILD_TIME=$(date)" might confuse in thinking that the quotes are passed down to bash. In this example they won't, but in CIBW_ENVIRONMENT: BUILD_TIME="$(date)" they do.

What is worse is that yaml double quotes process backslashes, unlike literal, single quotes, multiline strings. So with ENV: "VAR=foo\\bar" bash actually receives one \, whereas with ENV: VAR="foo\\bar" bash receives two.

I think that showing examples without the yaml double quotes makes them more readable and less puzzling about who consumes what quotes or backslash - see especially the last example, containing a multi-token string.

I have added a changeset to #767: let me know if that's useful, no problem if it isn't :)

@henryiii
Copy link
Contributor

CIBW_ENVIRONMENT: 'BUILD_TIME="$(date)"' would work, I'd think. But this is probably fine without the surrounding quotes - the main reason to do that is it triggers an incorrect yaml construction, such as a colon followed by a space, or a leading dash - neither of those at least are going to happen in this setting.

@dvarrazzo
Copy link
Contributor Author

dvarrazzo commented Jul 19, 2021

CIBW_ENVIRONMENT: 'BUILD_TIME="$(date)"' would work, I'd think. But this is probably fine without the surrounding quotes - the main reason to do that is it triggers an incorrect yaml construction, such as a colon followed by a space, or a leading dash - neither of those at least are going to happen in this setting.

I wondered about that too. In a belt-and-braces bash script I would use my_date="$(date)". what I was worried about were the spaces. It seems it works fine without:

$ export DATE=$(date) FOO=bar
$ echo $DATE
Mon 19 Jul 16:57:50 CEST 2021
$ echo $FOO
bar

so I went for a minimal example, but TBH I think CIBW_ENVIRONMENT: 'BUILD_TIME="$(date)"' is more higienic.

Do we want to add the quotes?

@dvarrazzo
Copy link
Contributor Author

Do we want to add the quotes?

I have noticed that in the toml version you do have quotes around $(date) so I will add them to the yaml version too.

@henryiii
Copy link
Contributor

FYI, TOML always requires quotes. You can't leave a string unquoted.

@henryiii
Copy link
Contributor

(Variables are optional, "x" = and x = are equivalent unless you have a space or dot in the name. But but values must always have quotes, otherwise it must be a number, boolean, etc)

@phoerious
Copy link

Is there any way we can suppress backslash evaluation? I have to paths that I need to set on Windows and in my pyproject.toml, I now have this, which looks kind of ridiculous:

[tool.cibuildwheel.windows.environment]
INCLUDE = "C:\\\\vcpkg\\\\installed\\\\x64-windows\\\\include"
LIB = "C:\\\\vcpkg\\\\installed\\\\x64-windows\\\\lib"

One escape pass is required by TOML itself, the other by cibuildwheel, otherwise the path will end up as C:vcpkginstalledx64-windowsinclude.

@henryiii
Copy link
Contributor

henryiii commented Mar 7, 2023

Use single quotes, that will remove one layer of backslashes (the TOML one).

@phoerious
Copy link

But then there is still the inconsistent backslash handling of cibuildwheel. In CIBW_REPAIR_WHEEL_COMMAND_WINDOWS it's sufficient to use literal backslashes, but here I have to escape them.

@henryiii
Copy link
Contributor

henryiii commented Mar 7, 2023

I rather expect we shouldn't be processing backslashes, sounds a bit like a bug (and TOML already supports them doing various escapes when using "" instead of ''). .format doesn't, so I'm not sure what we are doing that's processing an extra set.

@joerick
Copy link
Contributor

joerick commented Mar 7, 2023

The backslash escaping requirement is a result of the bash expansion, which lets you do stuff like:

[tool.cibuildwheel.macos.environment]
PATH = "$PATH:/your/dir/here"

However, if you wanted to set a variable to literally $reference, this wouldn't work:

[tool.cibuildwheel.windows.environment]
SOME_OPTION = "$reference"

because it's interpreted as an environment variable. In this case, you could do

[tool.cibuildwheel.windows.environment]
SOME_OPTION = '\$reference'

to get what you want.

That's why backslash is an escape character in this context. It would be nice if there was some way to set values as 'raw', though, that would disable the bash processing... maybe something in toml like

[tool.cibuildwheel.windows.environment]
SOME_OPTION.raw = "$reference"

or could be

[tool.cibuildwheel.windows.environment-raw]
SOME_OPTION = "$reference"

(...harder to make a environment variable version of this, though, since the string is already split by a bash lexer to separate the parts.)

@joerick
Copy link
Contributor

joerick commented Mar 7, 2023

But then there is still the inconsistent backslash handling of cibuildwheel. In CIBW_REPAIR_WHEEL_COMMAND_WINDOWS it's sufficient to use literal backslashes, but here I have to escape them.

I think that is the case for the windows shell, yeah. But only windows, for mac/linux you do have to escape them, as they run in a posix shell. Cross-platform is tricky 😣

@henryiii
Copy link
Contributor

henryiii commented Mar 7, 2023

Can't you just use forward slashes?

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 a pull request may close this issue.

4 participants