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

Fix platform compatibility for _os_find and _os_date functions #5655

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

rennsax
Copy link
Contributor

@rennsax rennsax commented Sep 24, 2024

This PR should close #5654.

  • Removed macOS-specific conditional checks in _os_find. Now using universal GNU find flag -maxdepth and -mindepth to simplify code. Note that -maxdepth and -mindepth are also available for macOS's find.
  • Updated _os_date to handle multiple platforms more gracefully by attempting GNU date options first, falling back to BSD date options, and finally reverting to the current date and time if none of the preceding options are available.

These changes ensure broader compatibility across different operating systems and utilities.

@rennsax
Copy link
Contributor Author

rennsax commented Sep 24, 2024

@waruqi 感谢 review

_os_find 调用比较多,而且 macOS find 事实上也支持 -maxdepth 和 -mindepth 选项,我直接统一了。

_os_date 我看只用到了一次,所以采用了 SOURCE_DATE_EPOCH — reproducible-builds.org 上的解决方法。

目前我在自己的 macOS 和 GNU/Linux 上测试过。可能不太完善,期待后续测试。

附:现在 $() 中的命令如果返回状态值不为零,都不会使 configure 脚本异常退出。后面如果有计划改进构建系统,可以考虑这个点,增加构建系统的鲁棒性。

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


@waruqi Thanks for review

_os_find is called a lot, and macOS find actually supports the -maxdepth and -mindepth options, so I unified them directly.

_os_date I saw was only used once, using the solution on SOURCE_DATE_EPOCH — reproducible-builds.org.

So far I've tested it on my own macOS and GNU/Linux. It may not be perfect, and I look forward to subsequent testing.

Attachment: Now if the return status value of the command in $() is not zero, the configure script will not exit abnormally. If you plan to improve the build system later, you can consider this point to increase the robustness of the build system.

else
_ret=$(find "${dir}" -maxdepth "${depth}" -mindepth "${depth}" -type f -name "${name}")
fi
_ret=$(find "${dir}" -maxdepth "${depth}" -mindepth "${depth}" -type f -name "${name}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我这里就不支持,macOS

$ find --maxpatch
find: illegal option -- -
usage: find [-H | -L | -P] [-EXdsx] [-f path] path ... [expression]
       find [-H | -L | -P] [-EXdsx] -f path [path ...] [expression]

Copy link
Contributor Author

@rennsax rennsax Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

一个杠呢?macOS find 应该不支持长选项

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

也不行,当初就是不支持,我才区分处理的

$ find -maxdepth
find: illegal option -- m
usage: find [-H | -L | -P] [-EXdsx] [-f path] path ... [expression]
       find [-H | -L | -P] [-EXdsx] -f path [path ...] [expression]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个是因为 macOS find 不支持忽略目录参数(紧跟着 find 命令的那个)。你可以试试:

/usr/bin/find . -maxdepth 1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

哦 这样可以

Copy link
Contributor Author

@rennsax rennsax Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

目录前面的全局参数,例如 -H, -L 是被特殊处理的。目录后面才可以用 -maxdepth 这样的过滤参数。GNU find 就没这个问题。

configure Outdated
else
_ret=$(date -u -d "@$SOURCE_DATE_EPOCH" +"${1}")
# Use GNU date options first, then fallback to BSD's, and finally fallback to current time.
_ret=$(date -u -d "@$SOURCE_DATE_EPOCH" +"${1}" || date -u -r "$SOURCE_DATE_EPOCH" +"${1}" || date +"${1}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ date -u -d "@$SOURCE_DATE_EPOCH" +"${1}" || date -u -r "$SOURCE_DATE_EPOCH" +"${1}" || date +"${1}"
date: illegal option -- d
usage: date [-jnRu] [-I[date|hours|minutes|seconds]] [-f input_fmt]
            [-r filename|seconds] [-v[+|-]val[y|m|w|d|H|M|S]]
            [[[[mm]dd]HH]MM[[cc]yy][.SS] | new_date] [+output_fmt]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不好意思,忘了重定向 stderr,这样会导致输出有错误,但是 _ret 拿到的东西是对的。

Copy link
Contributor Author

@rennsax rennsax Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

修改了,可以试试新的。

- Removed macOS-specific conditional checks in _os_find. Now using universal
  GNU find flag -maxdepth and -mindepth to simplify code. Note that -maxdepth
  and -mindepth are also available for macOS's find.
- Updated _os_date to handle multiple platforms more gracefully by attempting
  GNU date options first, falling back to BSD date options, and finally reverting
  to the current date and time if none of the preceding options are available.

These changes ensure broader compatibility across different operating systems
and utilities.
@waruqi waruqi added this to the v2.9.6 milestone Sep 24, 2024
@waruqi waruqi merged commit 583f8e6 into xmake-io:dev Sep 24, 2024
19 checks passed
@waruqi
Copy link
Member

waruqi commented Sep 24, 2024

thanks

@rennsax rennsax mentioned this pull request Sep 25, 2024
13 tasks
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.

_os_find 等函数应该判断工具本身的版本,而不是判断系统
3 participants