Page MenuHomePhabricator

thopre (Thomas Preud'homme)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 22 2017, 10:31 AM (165 w, 1 d)

Recent Activity

Today

thopre added a comment to D94766: Add lit env variable to disable indirect checks.

So the lit invocations are in a script of some sort, right? Such a script could perform a feature test to handle this use case: if --no-indirectly-run-check is supported, add it to some lit options variable used by all relevant lit invocations in the script, and do not otherwise. Is that difficult to do?

I'm not strongly opposed to this patch, especially if it helps many people. However, given that this use case is external and temporary, I'm inclined to think a feature test is more reasonable: it's also simple, but it's external like the use case and can be dropped by users when they decide they no longer need to support old lit versions. What do you think?

In our case lit is invoked for each testcase (1 lit invocation per testcase) and testcases are run in parallel. That would require detection before the tests but then you would need a fake test for that because lit does not have a mechanism to ask about whether an option exist except running a known good test with the option.

Yes, that's what I had in mind. It seems simple, but maybe I'm missing something.

A fake test is probably the most robust way to check. But an even simpler check that should work for now is: llvm-lit --help | grep -q -- --no-indirectly-run-check.

But all these are specifics to our setup (we have a workaround anyway), I'm more concerned about having a migration strategy.

What makes it specific to your setup? Feature tests when building/testing a project are common.

Thu, Jan 21, 2:25 PM · Restricted Project
thopre added a comment to D94766: Add lit env variable to disable indirect checks.

So the lit invocations are in a script of some sort, right? Such a script could perform a feature test to handle this use case: if --no-indirectly-run-check is supported, add it to some lit options variable used by all relevant lit invocations in the script, and do not otherwise. Is that difficult to do?

I'm not strongly opposed to this patch, especially if it helps many people. However, given that this use case is external and temporary, I'm inclined to think a feature test is more reasonable: it's also simple, but it's external like the use case and can be dropped by users when they decide they no longer need to support old lit versions. What do you think?

Thu, Jan 21, 12:31 PM · Restricted Project
thopre added a comment to D94766: Add lit env variable to disable indirect checks.

Can you explain the use case a bit more? I don't find myself moving between old and new versions of lit very often. Perhaps I'm naive, but it seems like it should be easy enough to just not specify --no-indirectly-run-check when using the old version.

Thu, Jan 21, 9:34 AM · Restricted Project
thopre added a reviewer for D94766: Add lit env variable to disable indirect checks: jdenny.

Ping?

Thu, Jan 21, 4:12 AM · Restricted Project

Yesterday

thopre closed D95069: Install future before running sphinx.
Wed, Jan 20, 10:43 AM
thopre committed rLNTf48d431f4461: Install future before running sphinx (authored by thopre).
Install future before running sphinx
Wed, Jan 20, 10:43 AM
thopre committed rLNT4d2d4dc93189: Fix mypy warning about short_help using __doc__ (authored by thopre).
Fix mypy warning about short_help using __doc__
Wed, Jan 20, 10:43 AM
thopre closed D95067: Fix mypy warning about short_help using __doc__.
Wed, Jan 20, 10:43 AM
thopre committed rLNT1a7257d91049: Fix type hint for add_column in lnt.server.db.util (authored by thopre).
Fix type hint for add_column in lnt.server.db.util
Wed, Jan 20, 10:41 AM
thopre closed D95064: Fix type hint for add_column in lnt.server.db.util.
Wed, Jan 20, 10:41 AM
thopre closed D95068: Help mypy with typing.
Wed, Jan 20, 10:39 AM
thopre committed rLNT001ac8fa222a: Help mypy with typing (authored by thopre).
Help mypy with typing
Wed, Jan 20, 10:39 AM
thopre committed rLNT38072912f259: Fix return type in baseline()'s type hint (authored by thopre).
Fix return type in baseline()'s type hint
Wed, Jan 20, 10:37 AM
thopre closed D95066: Fix return type in baseline()'s type hint.
Wed, Jan 20, 10:37 AM
thopre committed rLNTc8a6486626c8: Tell mypy to ignore import of cython module (authored by thopre).
Tell mypy to ignore import of cython module
Wed, Jan 20, 10:37 AM
thopre closed D95065: Tell mypy to ignore import of cython module.
Wed, Jan 20, 10:37 AM
thopre closed D94759: Mirror LLVM's arc config.
Wed, Jan 20, 10:32 AM
thopre committed rLNT94712399ab6b: Mirror LLVM's arc config (authored by thopre).
Mirror LLVM's arc config
Wed, Jan 20, 10:32 AM
thopre requested review of D95069: Install future before running sphinx.
Wed, Jan 20, 10:22 AM
thopre requested review of D95068: Help mypy with typing.
Wed, Jan 20, 10:12 AM
thopre requested review of D95067: Fix mypy warning about short_help using __doc__.
Wed, Jan 20, 10:11 AM
thopre requested review of D95066: Fix return type in baseline()'s type hint.
Wed, Jan 20, 10:11 AM
thopre requested review of D95065: Tell mypy to ignore import of cython module.
Wed, Jan 20, 10:10 AM
thopre requested review of D95064: Fix type hint for add_column in lnt.server.db.util.
Wed, Jan 20, 10:10 AM
thopre added a comment to D95033: Disable mypy cache.

Why does the cache cause this error in mypy? Is it a documented problem?

Wed, Jan 20, 5:07 AM
thopre requested review of D95033: Disable mypy cache.
Wed, Jan 20, 4:47 AM
thopre updated the diff for D94759: Mirror LLVM's arc config.

Add callsign now that LNT is in phabricator

Wed, Jan 20, 4:44 AM

Sat, Jan 16

thopre closed D94842: Extend mandatory Flake8 cleanness.
Sat, Jan 16, 12:24 PM
thopre closed D94837: Fix use of undefined session variable.
Sat, Jan 16, 12:23 PM
thopre added inline comments to D94837: Fix use of undefined session variable.
Sat, Jan 16, 10:55 AM
thopre closed D94841: Fix line too long Flake8 warning.
Sat, Jan 16, 6:27 AM
thopre closed D94840: Add import needed for typing hints.
Sat, Jan 16, 6:27 AM
thopre added a comment to D94840: Add import needed for typing hints.

was it being indirectly imported before?

Sat, Jan 16, 6:27 AM
thopre closed D94839: Fix OrderField's copy constructor.
Sat, Jan 16, 6:25 AM
thopre closed D94838: Remove nonsensical fct definition.
Sat, Jan 16, 6:24 AM
thopre updated the summary of D94837: Fix use of undefined session variable.
Sat, Jan 16, 6:23 AM

Fri, Jan 15

thopre requested review of D94842: Extend mandatory Flake8 cleanness.
Fri, Jan 15, 3:55 PM
thopre requested review of D94841: Fix line too long Flake8 warning.
Fri, Jan 15, 3:55 PM
thopre requested review of D94840: Add import needed for typing hints.
Fri, Jan 15, 3:55 PM
thopre requested review of D94839: Fix OrderField's copy constructor.
Fri, Jan 15, 3:54 PM
thopre requested review of D94838: Remove nonsensical fct definition.
Fri, Jan 15, 3:54 PM
thopre requested review of D94837: Fix use of undefined session variable.
Fri, Jan 15, 3:53 PM
thopre accepted D94767: [LNT] Fix some ResourceWarnings.

LGTM assuming tox is clean.

Fri, Jan 15, 11:20 AM
thopre closed D94799: Fix ambiguous variable names.
Fri, Jan 15, 11:16 AM
thopre updated the diff for D94799: Fix ambiguous variable names.

Rename variable in pairs() to avoid noqa hint

Fri, Jan 15, 10:33 AM
thopre closed D94791: Fix remaining Flake8 whitespace issues.
Fri, Jan 15, 9:45 AM
thopre closed D94797: Fix bracket indentation in report.py.
Fri, Jan 15, 9:44 AM
thopre closed D94794: Fix over indentation.
Fri, Jan 15, 9:44 AM
thopre closed D94793: Fix redundant backslash.
Fri, Jan 15, 9:41 AM
thopre closed D94792: Fix blank line errors.
Fri, Jan 15, 9:41 AM
thopre closed D94790: Fix space before typing comments.
Fri, Jan 15, 9:40 AM
thopre closed D94789: Fix unexpected space around keyword/param equals.
Fri, Jan 15, 9:32 AM
thopre added inline comments to D94799: Fix ambiguous variable names.
Fri, Jan 15, 9:31 AM
thopre added a comment to D94791: Fix remaining Flake8 whitespace issues.

This seems rather random compared with D94789... so the spacing rule isn't consistent for assignments?

Fri, Jan 15, 9:29 AM
thopre requested review of D94799: Fix ambiguous variable names.
Fri, Jan 15, 9:22 AM
thopre requested review of D94797: Fix bracket indentation in report.py.
Fri, Jan 15, 9:15 AM
thopre requested review of D94794: Fix over indentation.
Fri, Jan 15, 9:12 AM
thopre requested review of D94793: Fix redundant backslash.
Fri, Jan 15, 9:12 AM
thopre requested review of D94792: Fix blank line errors.
Fri, Jan 15, 9:11 AM
thopre requested review of D94791: Fix remaining Flake8 whitespace issues.
Fri, Jan 15, 9:09 AM
thopre requested review of D94790: Fix space before typing comments.
Fri, Jan 15, 9:08 AM
thopre requested review of D94789: Fix unexpected space around keyword/param equals.
Fri, Jan 15, 9:07 AM
thopre closed D94757: F841: local variable assigned to but never used.
Fri, Jan 15, 9:05 AM
thopre accepted D94775: [LNT] add __pycache__ to ignored files.
Fri, Jan 15, 6:46 AM
thopre accepted D94775: [LNT] add __pycache__ to ignored files.

LGTM

Fri, Jan 15, 6:45 AM
thopre accepted D94774: [LNT] Fix F811 warnings.

LGTM

Fri, Jan 15, 6:33 AM
thopre closed D94758: Ignore files generated by running tox.
Fri, Jan 15, 6:19 AM
thopre added inline comments to D94767: [LNT] Fix some ResourceWarnings.
Fri, Jan 15, 6:17 AM
thopre added inline comments to D94767: [LNT] Fix some ResourceWarnings.
Fri, Jan 15, 5:25 AM
thopre requested review of D94766: Add lit env variable to disable indirect checks.
Fri, Jan 15, 4:52 AM · Restricted Project
thopre requested review of D94759: Mirror LLVM's arc config.
Fri, Jan 15, 3:04 AM
thopre closed D94418: [LNT] Python 3 support: fix several bytes/str inconsistencies.

Landed as 80ed0db735b4636d40fb780605b9f947a19f7b7c

Fri, Jan 15, 2:55 AM
thopre accepted D94418: [LNT] Python 3 support: fix several bytes/str inconsistencies.
Fri, Jan 15, 2:54 AM
thopre added a comment to D94418: [LNT] Python 3 support: fix several bytes/str inconsistencies.

I'm personally more enclined to prevent lit from running without lit installed because then some of the tests are skipped.

Do you mean without lnt installed? If so, I agree. It makes sense to expect LNT to b fully configured/installed before running the tests.
I think the initial test skip was done in case cPerf fails to build: https://github.com/llvm/llvm-lnt/commit/a57bf295f7988de9d9f5fbf30b429cf03d54271d
Not sure if it's still relevant for supported platforms (does it build on Mac?). Even if it doesn't, no harm in having those tests failed there.

Not sure about PYTHONPATH manipulations. Go ahead, if it fixes running under tox.

I see you've already fixed the issue with LNT :: server/ui/V4Pages.py I had.
Is the buildbot still running for LNT? I feel uneasy seeing how many tests were silently failing.

Fri, Jan 15, 2:54 AM
thopre requested review of D94758: Ignore files generated by running tox.
Fri, Jan 15, 2:53 AM
thopre requested review of D94757: F841: local variable assigned to but never used.
Fri, Jan 15, 2:49 AM

Thu, Jan 14

thopre closed D94723: Fix under and over continuation indentation.
Thu, Jan 14, 2:08 PM
thopre closed D94722: Fix whitespaces flake8 errors and warnings.
Thu, Jan 14, 2:07 PM
thopre closed D94721: Fix multiple import on one line.
Thu, Jan 14, 2:07 PM
thopre closed D94720: Fix non multiple of 4 indentation.
Thu, Jan 14, 2:07 PM
thopre requested review of D94723: Fix under and over continuation indentation.
Thu, Jan 14, 1:55 PM
thopre requested review of D94722: Fix whitespaces flake8 errors and warnings.
Thu, Jan 14, 1:54 PM
thopre requested review of D94721: Fix multiple import on one line.
Thu, Jan 14, 1:54 PM
thopre requested review of D94720: Fix non multiple of 4 indentation.
Thu, Jan 14, 1:52 PM
thopre closed D94686: Fix all unused imports flake8 warnings.
Thu, Jan 14, 1:51 PM
thopre closed D94715: Expect LNT to be installed in the tests.
Thu, Jan 14, 1:46 PM
thopre updated the diff for D94686: Fix all unused imports flake8 warnings.

Rebase on top of D94715

Thu, Jan 14, 12:59 PM
thopre requested review of D94715: Expect LNT to be installed in the tests.
Thu, Jan 14, 12:56 PM
thopre updated the diff for D94686: Fix all unused imports flake8 warnings.

Explain reason for noqa inline

Thu, Jan 14, 12:33 PM
thopre added a comment to D94663: Fix use of wrong and undef variable.

Landed as 0d067a27868dae963ac9bc4a21e7da434822dd01

Thu, Jan 14, 11:58 AM
thopre closed D94663: Fix use of wrong and undef variable.
Thu, Jan 14, 11:57 AM
thopre closed D94635: Fix server/ui/V4Pages test.

Landed as 52ea66719d01d3d18651946c1b83790ca371d34c

Thu, Jan 14, 11:56 AM
thopre added a comment to D94418: [LNT] Python 3 support: fix several bytes/str inconsistencies.

Sorry, pushed to the wrong branch (Phabricator/arcanist workflow is annoying >_<) and didn't notice your comment.

Hm, I've made sure that all tests pass on both py2 and py3 before submitting and I can't reproduce it on my machine.
Could you check if test_suite-profile-import.py.tmp.err and test_suite-profile-import.py.tmp.log have any suspicious errors (they should be in <..>/test_run_tmp/runtest/Output/)?

%t.err has:

2021-01-14 18:07:42 WARNING: Traceback (most recent call last):

File "/home/thomasp/repos/lnt/lnt/testing/profile/perf.py", line 32, in deserialize
  data = cPerf.importPerf(f, nm, objdump)

NameError: global name 'cPerf' is not defined

I'm not sure why cPerf appears not to be installed under tox, it should build everything. tests/testing/cPerf.py does not have this problem because it skips the test if cPerf is not found.

Thu, Jan 14, 11:51 AM
thopre added a comment to D94418: [LNT] Python 3 support: fix several bytes/str inconsistencies.

Sorry, pushed to the wrong branch (Phabricator/arcanist workflow is annoying >_<) and didn't notice your comment.

Hm, I've made sure that all tests pass on both py2 and py3 before submitting and I can't reproduce it on my machine.
Could you check if test_suite-profile-import.py.tmp.err and test_suite-profile-import.py.tmp.log have any suspicious errors (they should be in <..>/test_run_tmp/runtest/Output/)?

Thu, Jan 14, 10:13 AM
thopre added a comment to D94418: [LNT] Python 3 support: fix several bytes/str inconsistencies.

Sorry, pushed to the wrong branch (Phabricator/arcanist workflow is annoying >_<) and didn't notice your comment.

Hm, I've made sure that all tests pass on both py2 and py3 before submitting and I can't reproduce it on my machine.
Could you check if test_suite-profile-import.py.tmp.err and test_suite-profile-import.py.tmp.log have any suspicious errors (they should be in <..>/test_run_tmp/runtest/Output/)?

Do you use tox to run the tests?

I use lit directly (lit -svv ./tests) which what I understand tox does as well. I've rerun all tests using tox and the only test that failed was server/ui/V4Pages.py (I believe it's because I've already had an lnt instance running and that test doesn't account for that possibility).

Thu, Jan 14, 9:31 AM
thopre added a comment to D94418: [LNT] Python 3 support: fix several bytes/str inconsistencies.

Sorry, pushed to the wrong branch (Phabricator/arcanist workflow is annoying >_<) and didn't notice your comment.

Hm, I've made sure that all tests pass on both py2 and py3 before submitting and I can't reproduce it on my machine.
Could you check if test_suite-profile-import.py.tmp.err and test_suite-profile-import.py.tmp.log have any suspicious errors (they should be in <..>/test_run_tmp/runtest/Output/)?

Thu, Jan 14, 8:09 AM
thopre added a comment to D94418: [LNT] Python 3 support: fix several bytes/str inconsistencies.

I've tried applying your patch and its test fail for me.

Thu, Jan 14, 7:01 AM
thopre requested changes to D94418: [LNT] Python 3 support: fix several bytes/str inconsistencies.

I've tried applying your patch and its test fail for me.

Thu, Jan 14, 7:00 AM
thopre added a comment to D94418: [LNT] Python 3 support: fix several bytes/str inconsistencies.

I've seen it landed on the upstream branch rather than the main branch. Was that a mistake?

Thu, Jan 14, 6:58 AM
thopre requested review of D94686: Fix all unused imports flake8 warnings.
Thu, Jan 14, 6:54 AM