-
-
Notifications
You must be signed in to change notification settings - Fork 158
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
spec tests should run in a known environment #42
Comments
14 problems on Ubuntu 17 due to bash/mksh/etc. version mismatches :-(
|
Labelling this help wanted because maybe someone can write a shell script to make a chroot image? I can provide more color on this. |
@Yorwba Let's continue the discussion here. I said Alpine might be harder and Ubuntu is probably easier. The reason I think that is:
So potentially every test has to be tweaked. But maybe this is not as big a deal as I thought. Intermediate task (which I may pick up): Provide an alias to run a single spec test in a chroot. Then we can migrate it test-by-test. You know I think the main issue is: what shells do we test against?
I was thinking #3 because it gives us the most control over what shell version. Although honestly I think "whatever shell version the latest LTS version of Ubuntu uses" is not bad. I was thinking "the latest version of every shell", but in theory that is more work (though it shouldn't be THAT much work.) I have to go back and remember how debootstrap works. If I can debootstrap Ubuntu 18.04 from my 16.04 machine, that would be nice. But I think Ubuntu doesn't necessarily do that. In other words, I don't want the version of shells we test against to be tied to "did Andy upgrade his personal machine, or does he have access to an 18.04 machine?" Right now I do not. Alpine is nice because they provide a small chroot image to download. And it seems to run fine on Ubuntu. So there's less possibility of "hidden" dependencies between guest and host. |
Thinking aloud here, another reason I have put this is off is because Ubuntu doesn't provide a nice root file system download like Alpine: https://alpinelinux.org/downloads/ On Ubuntu you have to run debootstrap yourself and do some other nontrivial setup steps, as far as I remember. I also have to check out what versions of dash/mksh/zsh/busybox Alpine has, if any. |
It seems like using Alpine's edge branch should keep you pretty close to "the latest version of every shell". E.g. the repository currently has bash 4.4.19, which was packaged the day after the upstream release. The highest patchlevel would be 4.4.23, but judging by the upload date in GNU's FTP, those patches are barely over a week old. Looking through the packages for the other shells, the delay never seems to be more than a few weeks. |
Small update on this:
The downside is that this method might require everyone to build 4 or 5 shells from scratch. Or maybe I can build some "portable" binaries. I think I read about a blog post about that, which basically amounted to using an old libc (?) https://www.cyphar.com/blog/post/20160627-rootless-containers-with-runc |
Right now they are the versions that come with Ubuntu 16.04. test/spec.sh: If possible, use these hermetic shell binaries for spec tests. Addresses issue #42. I'm not sure why, but I had to adjust two tests. glob.test.sh: Upstream dash 0.5.8 also has the same bug as mksh. I'm not sure why the Ubuntu package, which is marked 0.5.8, doesn't have it. Maybe they ported a patch over? quote.test.sh: hermetic ash doesn't have this bug for some reason. Also: the regex test is currently failing for ZSH because we're not building with regex support. To be fixed.
@granttrec found out that while the shell binaries are now isolated, the environment isn't! The unicode tests depend on certain environment variables. I honestly forget all the details, but I know that both We could probably just set the environment variables in |
@andychu setting the environment variables will work, however we will have to check for a correct locale on the system first, then set |
OK, I'll leave it up to you if you want to submit a patch. If someone else runs into then it may become higher priority. If you do, please mention the tests that depend on the locale in the comments, and point to any relevant libc or bash documentation. |
In addition to $REPO_ROOT/spec/bin, we only need /usr/bin and /bin to run! This was motivated by a machine that somehow had '::' in its $PATH, which made spec/builtin-eval-source fail because the '.' would respect the current directory. Addresses issue #42.
Done with "toil" on Travis CI! https://github.com/oilshell/oil/tree/master/services |
There have been some issues with assuming /usr/bin/time, /usr/bin/python, /bin/bash, etc. This seems to be the first issue everybody runs into when developing Oil.
The tests also depend on gawk I think. And the shells: busybox, dash, bash, ash, etc.
Right now I think the best solution is an Alpine Linux chroot, because it's small and easy to set up. Should be runnable on any distro quite easily.
FYI @lheckemann
Also see issue #12
The text was updated successfully, but these errors were encountered: