This is an archive of the discontinued LLVM Phabricator instance.

[not][test] Fix disable-symbolization.test when 'printenv' is not available
ClosedPublic

Authored by MaskRay on Aug 24 2020, 4:59 PM.

Details

Summary

On Windows, 'env' or 'printenv' may not exist.

Also switch back to 'env' which is specified by POSIX.1-2017. 'printenv' is not
standard (I picked it because 'printenv' exists on GnuWin32 but 'env' does not).

Diff Detail

Event Timeline

MaskRay created this revision.Aug 24 2020, 4:59 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 24 2020, 4:59 PM
MaskRay requested review of this revision.Aug 24 2020, 4:59 PM
zequanwu accepted this revision.Aug 24 2020, 5:06 PM

LGTM.

This revision is now accepted and ready to land.Aug 24 2020, 5:06 PM
MaskRay updated this revision to Diff 287529.Aug 24 2020, 5:21 PM

Add a comment

MaskRay updated this revision to Diff 287530.Aug 24 2020, 5:25 PM
MaskRay edited the summary of this revision. (Show Details)

Improve description

MaskRay updated this revision to Diff 287531.Aug 24 2020, 5:28 PM

Use -s to suppress 'cmp' output

This revision was landed with ongoing or failed builds.Aug 24 2020, 5:29 PM
This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.Aug 24 2020, 6:44 PM

This breaks check-llvm on windows: http://45.33.8.238/win/22625/step_11.txt

Please take a look and revert for now if it takes a while to fix.

(I'm guessing it was failing under cmd previously and fails in msys/gig bash now?)

I'll fix it.

MaskRay added a comment.EditedAug 24 2020, 6:55 PM

This breaks check-llvm on windows: http://45.33.8.238/win/22625/step_11.txt

Please take a look and revert for now if it takes a while to fix.

(I'm guessing it was failing under cmd previously and fails in msys/gig bash now?)

It worked in a GnuWin32 environment previously but a Chromium (which did not install full GnuWin32) bot was unhappy. So I created this patch. Glad that Alexandre will handle it. Thanks!
Looks like GnuWin32 'env' is not conformant...

This breaks check-llvm on windows: http://45.33.8.238/win/22625/step_11.txt

Please take a look and revert for now if it takes a while to fix.

(I'm guessing it was failing under cmd previously and fails in msys/gig bash now?)

It worked in a GnuWin32 environment previously but a Chromium (which did not install full GnuWin32) bot was unhappy. So I created this patch. Glad that Alexandre will handle it. Thanks!
Looks like GnuWin32 'env' is not conformant...

It is not, GnuWin32 comes with CoreUtils 5.3 from 2005 I believe.

@thakis Do you think you can add printenv.exe into https://commondatastorage.googleapis.com/chromium-browser-clang/tools/gnuwin-XX.zip so we can enable the test again on Windows? Please see https://reviews.llvm.org/D86170#inline-799301

We have env as a lit builtin. Can we just use that? What's the not --crash supposed to do here? Why would env crash anyway? I miss context on what this test wants to do.

@thakis Do you think you can add printenv.exe into https://commondatastorage.googleapis.com/chromium-browser-clang/tools/gnuwin-XX.zip so we can enable the test again on Windows? Please see https://reviews.llvm.org/D86170#inline-799301

We have env as a lit builtin. Can we just use that? What's the not --crash supposed to do here? Why would env crash anyway? I miss context on what this test wants to do.

I was under the false impression yesterday that the local env.exe (found in GnuWin32) wouldn't print the environment block, but it does. It seems the problem lies in how the commands are interpreted by lit. On Windows, with this test, we reach this: https://github.com/llvm/llvm-project/blob/master/llvm/utils/lit/lit/TestRunner.py#L645 - but on not Linux?