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

Env variables with different cases #3752

Closed
xq114 opened this issue May 17, 2023 · 12 comments
Closed

Env variables with different cases #3752

xq114 opened this issue May 17, 2023 · 12 comments
Labels
Milestone

Comments

@xq114
Copy link
Contributor

xq114 commented May 17, 2023

Xmake 版本

2.7.9

操作系统版本和架构

Windows 11

描述问题

image

如图所示,xmake中os.getenvs()返回的环境变量全为大写,而实际存在的一些环境变量是有小写的。如果不合并,在使用os.exec()的时候就会存在问题,见 https://stackoverflow.com/questions/75445876/cannot-set-path-in-windows-from-boostprocess/75446143#75446143 。这是因为win32 api的CreateProcessW中lpEnvironment是区分大小写的。

已经导致的问题:例如 #3124 package.tools.cmake.buildenvs(package)生成了不合法的环境变量。

期待的结果

最好的办法是getenv()能获取到正确大小写的环境变量。win32 GetEnvironmentStrings返回的环境变量是区分大小写的,但不知道为什么xmake里就变成全大写了。

如果不修改getenv(),xmake内部可以修改os.joinenv()来处理一部分情况,但xmake中用到os.getenvs和os.setenv的地方不止这一处,很有可能在别的地方仍有隐患。可以在os.execv加一个检查,如果存在字符相同但大小写不同的环境变量就报错。

工程配置

No response

附加信息和错误日志

No response

@xq114 xq114 added the bug label May 17, 2023
@xq114
Copy link
Contributor Author

xq114 commented May 17, 2023

xmake-io/xmake-repo#2076 也疑似有关

@waruqi
Copy link
Member

waruqi commented May 18, 2023

最好的办法是getenv()能获取到正确大小写的环境变量。win32 GetEnvironmentStrings返回的环境变量是区分大小写的,但不知道为什么xmake里就变成全大写了。

我不记得有对 name 特殊处理过改成大小,目前看源码,就是直接取了 GetEnvironmentStrings 返回,没做处理。。这应该 win env api 自动转了吧。。

而且win 下环境变量 name 区分大小写反而问题更多。。比如 vs 环境 xmake 获取的是 WindowsSDKVersion 。。但是有些用户和 ci 环境,或者 cmd 环境,额外设置了全大写的。。WINDOWSSDKVERSION,导致 xmake 里面调用 cmake 编译 vs 项目,报各种 envs 冲突错误,存在大小写两个 windows sdk version。。msbuild 直接报错了。。

@waruqi
Copy link
Member

waruqi commented May 18, 2023

哦 找到了,确实里面转了。。

key = key:upper()

但是我忘记当初为什么要加了。。应该也是趟了坑 才加的

@waruqi
Copy link
Member

waruqi commented May 18, 2023

好像是因为,尽管 GetEnvironmentStrings 区分大小写。。但是 win 的 set env 不区分,如果不转大小,获取 env 会有问题。

比如

Y:\personal\xmake>set AAA=AAA

Y:\personal\xmake>set aaa=aaa

Y:\personal\xmake>echo %AAA%
aaa

Y:\personal\xmake>echo %aaa%
aaa

最后实际设置到 envs 的只有最后设置的 aaa

也就是说 win 的 envs 设置本身就是不区分大小写的。。

如果我 getenvs 里面不转。。那么

set path=xxxx

getenvs 只能取到 envs.path 。。但是 lua 的 table 是区分大小写的。。导致脚本里去访问 envs.PATH 就返回空了。。各种逻辑都会失败。。

所以当初转成大写,统一下格式,相当于 xmake 内部 envs 获取做了合并

@waruqi
Copy link
Member

waruqi commented May 18, 2023

另外即使我对 getenvs 去掉 大小写转换。。如果你同时设置了 set AAA=aaaset aaa=aaa ,那么 GetEnvironmentStrings 只会获取到 AAA=aaa,没有 aaa=aaa ,不会两个同时存在,GetEnvironmentStrings 内部似乎也作了去重合并,并且仅返回大写版本。。AAA

只有当 仅仅设置了 bbb=bbb ,没有大写 BBB 的 key 设置存在,GetEnvironmentStrings 才会返回对应 bbb=bbb 小写版本

@xq114
Copy link
Contributor Author

xq114 commented May 18, 2023

但是 win 的 set env 不区分,如果不转大小,获取 env 会有问题。

是这样的,windows上setEnvVar和单独的getEnvVar都是不区分大小写的,但CreateProcess传入的env和GetEnvStrings获取的env区分大小写。单独setenv和getenv,如果存在只有大小写不同的环境变量,那windows会自动去设置/获取已有的环境变量,不会新建。

但是有些用户和 ci 环境,或者 cmd 环境,额外设置了全大写的。。WINDOWSSDKVERSION

同上,用户无法设置全大写的WINDOWSSDKVERSION,因为vs powershell/cmd设置了WindowsSDKVersion。之后用户对WINDOWSSDKVERSION的所有访问都会被windows自动转成对WindowsSDKVersion的访问。除非用户无聊,故意调用CreateProcess去卡bug设置两个环境变量,或者不用vs提供的环境自己手动一个一个加大写环境变量,但我觉得这个情况不需要支持。现在出问题是因为xmake中getenv()返回了一个大写WINDOWSSDKVERSION,然后调用CreateProcess去创建进程的时候就炸了

导致脚本里去访问 envs.PATH 就返回空了

Windows上官方的路径变量是Path而不是PATH;按现在的实现,脚本去访问envs.Path也会返回空。如果要彻底解决这个问题,需要设计一个env类,调用env.get('PATH')来获取环境变量。如果只是为了兼容,可以只把Path变量转成大写和UNIX保持一致,其他变量原样不变,这样也可以避免现在的问题。

如果你同时设置了 set AAA=aaa 和 set aaa=aaa ,那么 GetEnvironmentStrings 只会获取到 AAA=aaa,没有 aaa=aaa

想同时设置AAA和aaa必须且只能通过win32 api CreateProcess去创建进程才能实现,用cmd和powershell是实现不了的,只要定义了AAA就无法再定义aaa了。可以安全地假设不存在一个变量同时有大小写的情况,因为用户实现不了

@waruqi
Copy link
Member

waruqi commented May 18, 2023

除非用户无聊,故意调用CreateProcess去卡bug设置两个环境变量,或者不用vs提供的环境自己手动一个一个加大写环境变量

之前遇到过类似问题,用户吧自己系统 envs 设置里面把对应大写版本删了,才跑通。。

Windows上官方的路径变量是Path而不是PATH;按现在的实现,脚本去访问envs.Path也会返回空。如果要彻底解决这个问题,需要设计一个env类,调用env.get('PATH')来获取环境变量。如果只是为了兼容,可以只把Path变量转成大写和UNIX保持一致,其他变量原样不变,这样也可以避免现在的问题。

所以当前是通过转成大写来解决这个问题,不管外面哪里设置的 Path, path, PATH。xmake 里 getenvs 统一获取 envs.PATH 。。内部脚本统一使用 PATH,也就不会乱了。。

按现在的实现,脚本去访问envs.Path也会返回空

现在的实现不会,因为内部规范约定,只访问大写,不存在访问小写情况

而 getenv 直接走的 win api,即使访问 PATH, Path, path 还是能取到,不存在问题

@xq114
Copy link
Contributor Author

xq114 commented May 18, 2023

之前遇到过类似问题,用户吧自己系统 envs 设置里面把对应大写版本删了,才跑通。。

这个是应该的,用户不应该在系统乱设置同名不同大小写的环境变量,就算xmake兼容了,他用cmake或者别的应用还是会出错。可以加个检测就是了

所以当前是通过转成大写来解决这个问题,不管外面哪里设置的 Path, path, PATH。

这里确实应该转,因为UNIX上PATH和Windows上Path有相同含义,统一转成PATH可以同时兼容Windows和UNIX。但其一,在UNIX和Windows有相同含义且大小写不同的环境变量只有Path一个;其二,xmake中搜索envs\.[A-Z]+,用到的环境变量中原来是小写需要xmake转成大写的环境变量也只有Path一个。同时,xmake中detect vs/icc的环境变量仍是用的小写,例如WindowsSdkDir。

我觉得这里只将Path转成大写是合理的。这样还有额外的好处,就是通过xmake调用其他第三方软件时更不容易出问题,因为原来系统环境变量大小写不变,即使第三方软件对大小写问题处理不佳,但只要他能在系统环境跑,xmake就能正常调用。

@waruqi
Copy link
Member

waruqi commented May 18, 2023

这个是应该的,用户不应该在系统乱设置同名不同大小写的环境变量,就算xmake兼容了,他用cmake或者别的应用还是会出错。可以加个检测就是了

目前也没兼容,得用户自己删

xmake中搜索envs.[A-Z]+,用到的环境变量中原来是小写需要xmake转成大写的环境变量也只有Path一个

因为现在只遇到 PATH ,但也可能有其他的,只是暂时只遇到这一个。

xmake中detect vs/icc的环境变量仍是用的小写,例如WindowsSdkDir。

这个完全不走 getenvs 的,直接 bat 取的,跟 getenvs 不想关,两个问题。。现在的约定是,如果是从 getenvs 取出来的,都走大写访问

我觉得这里只将Path转成大写是合理的。这样还有额外的好处,就是通过xmake调用其他第三方软件时更不容易出问题,因为原来系统环境变量大小写不变,即使第三方软件对大小写问题处理不佳,但只要他能在系统环境跑,xmake就能正常调用。

仅处理 Path,反而各种硬编码,且不能保证其他是否存在类似问题。

@xq114
Copy link
Contributor Author

xq114 commented May 18, 2023

仅处理 Path,反而各种硬编码,且不能保证其他是否存在类似问题。

只有一处硬编码还好,而且 Path 变量确实特殊。转大写最主要解决的问题就是 win 和 unix 不一致问题,其他地方无论用户设置PATH path Path还是PatH,xmake 这里能取到的也只有 Path ,所以不太可能有其他变量一定需要转大写才能处理的情况。只是为了标准化的话,以系统自带为标准就行了,并且对UNIX也通用

现在是其他变量转大写已经导致问题了,需要修复了,不修这里,也得修别的地方,而且每个合并环境变量的地方都要改动,还要对win和unix区别判断(unix环境变量本来就分大小写),工作量更大了

@xq114
Copy link
Contributor Author

xq114 commented May 18, 2023

2c52aa3

看commit是刚支持windows就加了转大写,并不是遇到问题后加的,这里没有什么历史问题

@waruqi
Copy link
Member

waruqi commented May 18, 2023

可以先这么试下,后续持续关注下是否有其他异常。。#3756

@waruqi waruqi added this to the v2.8.1 milestone May 18, 2023
waruqi added a commit that referenced this issue May 18, 2023
@waruqi waruqi closed this as completed May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants