Skip to content
This repository was archived by the owner on Jun 16, 2025. It is now read-only.

Enable test cases for standard commands#500

Merged
siteshwar merged 5 commits intoatt:masterfrom
siteshwar:gh499
May 5, 2018
Merged

Enable test cases for standard commands#500
siteshwar merged 5 commits intoatt:masterfrom
siteshwar:gh499

Conversation

@siteshwar
Copy link
Copy Markdown
Contributor

No description provided.

siteshwar added 3 commits May 2, 2018 21:25
Since these builtins are now built by default we should enable their
test cases.

Related: #499
chmod builtin uses non standard flags provided by libast code. We should
use the default flags provided by system libraries.

Related: #499
in several other places.

Related: #499
@siteshwar
Copy link
Copy Markdown
Contributor Author

It fails to compile on opensuse because it uses an older version of glibc:

/usr/include/fts.h:41:3: error: #error "<fts.h> cannot be used with -D_FILE_OFFSET_BITS==64"
 # error "<fts.h> cannot be used with -D_FILE_OFFSET_BITS==64"

This error was removed and support for large files was added through this commit in glibc. But for now I think we should disable _FILE_OFFSET_BITS macro in our builds.

@siteshwar
Copy link
Copy Markdown
Contributor Author

I think this should be fixed in Meson. I have opened an issue for it mesonbuild/meson#3519

Older versions of glibc do not have support for -D_FILE_OFFSET_BITS=64
in fts functions. This flag is enabled by default in all meson builds.
Add a feature test for this macro and disable it if fts functions do not
support it.
@krader1961
Copy link
Copy Markdown
Contributor

Fails with the following one or both of the following errors on macOS, opensuse and ubuntu 16.04:

<E> builtins[811]: basename bound to /bin/basename but should be bound to /usr/bin/basename
<E> builtins[812]: cmp bound to /bin/cmp but should be bound to /usr/bin/cmp

@siteshwar
Copy link
Copy Markdown
Contributor Author

@krader1961 Fixed.

If .sh.op_astbin is changed, shell builtins will be bound to path
specified by it. This test case fails on systems that do not have cmp
and basename under /bin directory, so fix it's expected output.

Related: #499
@krader1961
Copy link
Copy Markdown
Contributor

LGTM, in as much as it no longer introduces new test failures on the platforms I'm testing against 😄 Also, I have no objection to the text of the change. But I'm far from happy about how builtin commands of this sort are exposed and tested. That, however, is a discussion for another day.

@siteshwar siteshwar merged commit 9274590 into att:master May 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants