Skip to content

vcpkg ci: Parse ci.baseline.txt to determine regressions#345

Merged
BillyONeal merged 30 commits intomicrosoft:mainfrom
autoantwort:parse-ci.baseline.txt
Mar 24, 2022
Merged

vcpkg ci: Parse ci.baseline.txt to determine regressions#345
BillyONeal merged 30 commits intomicrosoft:mainfrom
autoantwort:parse-ci.baseline.txt

Conversation

@autoantwort
Copy link
Contributor

@autoantwort autoantwort commented Feb 7, 2022

Probably you don't want to use iostreams. Should I add write_unlocalized_text_to_stderr to write to stderr?

This makes afaik the whole powershell parsing stuff in the ci obsolete

@BillyONeal
Copy link
Member

Stop doing things on my todo list before I do them 😅

Thanks for the submission! Will take a look soon

@autoantwort autoantwort force-pushed the parse-ci.baseline.txt branch from 4936f38 to a555645 Compare February 21, 2022 13:01
@strega-nil-ms
Copy link
Contributor

https://gist.githubusercontent.com/strega-nil-ms/03628b7f6febec5ff5355be78167593d/raw/8d7aab62a8872244ea71e5fae11ecd402f36f2e1/0001-Nicole-CRs.patch

My CRs are given in the patch above ^

@autoantwort autoantwort force-pushed the parse-ci.baseline.txt branch from a365c6d to fc2cbb5 Compare March 6, 2022 13:00
case Build::BuildResult::FILE_CONFLICTS:
if (expected_failures.find(port_result.spec) == expected_failures.end())
{
std::cerr
Copy link
Member

Choose a reason for hiding this comment

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

I think this should also use print2, and we certainly should avoiding new use of iostreams if at all possible.

@BillyONeal
Copy link
Member

Sorry it has taken me so long to get to this; I didn't leave comments back at the time because I thought I was going to respond quickly with the bits I wanted fixed but that didn't work out.

It looks like Nicole already commented about effectively everything I was going to say, except I would like to see a unit test put around parse_ci_baseline. At that point I would be happy to merge this and make the corresponding infrastructure changes.

If don't want to do that because you've been kept hanging too long I understand :)

@BillyONeal
Copy link
Member

If don't want to do that because you've been kept hanging too long I understand :)

Are you interested in that? Otherwise I can push that into this branch.

@autoantwort
Copy link
Contributor Author

Otherwise I can push that into this branch.

That would be cool. I am currently writing exams and don't have that much time :)

@BillyONeal
Copy link
Member

OK I have to fix the bug where the "one liner" doesn't update correctly and this is next on my list.

* Delete namespace Parse::
* Add forwarding headers for a lot of the parse machinery.
* Extract some supporting structures for parsing ci.baseline.txt into a header so that tests can be added.
# Conflicts:
#	include/vcpkg/base/parse.h
#	src/vcpkg/versions.cpp
Extract exclusions / ci baseline infrastructure.
Comment on lines +107 to +109
Checks::check_exit(VCPKG_LINE_INFO, equal_loc != std::string::npos, "Line '%s' must contain a '='", line);
parsed_line.triplet_name =
Strings::trim(StringView{line.data() + colon_loc + 1, line.data() + equal_loc}).to_string();
Copy link
Member

@BillyONeal BillyONeal Mar 18, 2022

Choose a reason for hiding this comment

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

FYI I'm changing this behavior because it fails, for example, with

catch-classic:arm64-windows      = skip
catch-classic:arm-uwp            = skip
catch-classic:x64-linux          = skip
catch-classic:x64-osx            = skip
catch-classic:x64-uwp            = skip
catch-classic:x64-windows        = skip
catch-classic:x64-windows-static = skip
catch-classic:x64-windows-static-md=skip
catch-classic:x86-windows        = skip

…rs_or_warnings, merge match_zero_or_more and match_until as merge_while, delocalize locale-invariant FormattedParseMessageLocation, start implementing actual parsing with ParserBase.
Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

Whoops, I forgot to get rid of cerr

"ChecksUnreachableCode": "unreachable code was reached",
"ChecksUpdateVcpkg": "updating vcpkg by rerunning bootstrap-vcpkg may resolve this failure.",
"CiBaselineRegression": "REGRESSION: {spec} failed with {build_result}. If expected, add {spec}=fail to {path}.",
"_CiBaselineRegression.comment": "example of {spec} is 'zlib:x64-windows'.\nexample of {build_result} is 'One of the BuildResultXxx messages (such as BuildResultSucceeded/SUCCEEDED)'.\nexample of {path} is '/foo/bar'.\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be {Locked}.

Copy link
Member

Choose a reason for hiding this comment

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

I wasn't sure if it should be. The "spec=fail" part needs to be but there's no reason the rest of the message couldn't be localized.

Copy link
Contributor

Choose a reason for hiding this comment

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

mmh, okay

for (;;)
{
auto n = match_until([](char32_t ch) { return ch == ',' || ch == '`' || ch == ';'; });
auto n = match_while([](char32_t ch) { return ch != ',' && ch != '`' && ch != ';'; });
Copy link
Contributor

Choose a reason for hiding this comment

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

why this change?

Copy link
Member

Choose a reason for hiding this comment

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

To merge match_until and match_zero_or_more into one function.

Copy link
Member

Choose a reason for hiding this comment

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

Reverted.

BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this pull request Mar 21, 2022
In microsoft#345 (comment) Nicole indicated that the workaround here to support features with _ in their names is no longer relevant because the feature in question was already renamed before versioning.

Since this is a change in the usable surface of the tool I extracted the change into its own more targeted PR.
BillyONeal added a commit that referenced this pull request Mar 22, 2022
In #345 (comment) Nicole indicated that the workaround here to support features with _ in their names is no longer relevant because the feature in question was already renamed before versioning.

Since this is a change in the usable surface of the tool I extracted the change into its own more targeted PR.
# Conflicts:
#	locales/messages.en.json
#	locales/messages.json
#	src/vcpkg/export.prefab.cpp
#	src/vcpkg/packagespec.cpp
BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this pull request Mar 22, 2022
Extracted from microsoft#345 to make that PR easier to review.
@BillyONeal
Copy link
Member

I sucked out #447 "Delete Parse::" to make this easier to review if you want to land that one first.

BillyONeal added a commit that referenced this pull request Mar 22, 2022
* Delete Parse::.

Extracted from #345 to make that PR easier to review.

* ... and IParseError, as requested by Nicole
# Conflicts:
#	include/vcpkg-test/util.h
#	include/vcpkg/base/fwd/parse.h
#	include/vcpkg/base/json.h
#	include/vcpkg/base/parse.h
#	src/vcpkg/base/json.cpp
#	src/vcpkg/base/parse.cpp
#	src/vcpkg/packagespec.cpp
Copy link
Contributor

@strega-nil strega-nil left a comment

Choose a reason for hiding this comment

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

Approved, but I still prefer to have match_until as well.

"ChecksUnreachableCode": "unreachable code was reached",
"ChecksUpdateVcpkg": "updating vcpkg by rerunning bootstrap-vcpkg may resolve this failure.",
"CiBaselineRegression": "REGRESSION: {spec} failed with {build_result}. If expected, add {spec}=fail to {path}.",
"_CiBaselineRegression.comment": "example of {spec} is 'zlib:x64-windows'.\nexample of {build_result} is 'One of the BuildResultXxx messages (such as BuildResultSucceeded/SUCCEEDED)'.\nexample of {path} is '/foo/bar'.\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

mmh, okay

@BillyONeal
Copy link
Member

Can we inherit from Compare then?

I really don't want to do that because it gives SortedVector an operator(). I can see an argument for a compressed_pair optimization in the future in order to implement this but I think that is a separate improvement and we shouldn't block over that.

Alternately, we can change the SortedVector design such that the caller always needs to pass the comparison functor every time they do something that needs it.... let me try that.

Copy link
Contributor

@strega-nil strega-nil left a comment

Choose a reason for hiding this comment

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

The one I want changed is try_match_keyword.

@BillyONeal BillyONeal merged commit 2a984ce into microsoft:main Mar 24, 2022
@BillyONeal
Copy link
Member

Thanks @autoantwort :D

BillyONeal added a commit to BillyONeal/vcpkg that referenced this pull request Mar 25, 2022
BillyONeal added a commit to microsoft/vcpkg that referenced this pull request Mar 28, 2022
* Update vcpkg-tool to 2022-03-24

* Hook up microsoft/vcpkg-tool#345

* Hook up microsoft/vcpkg-tool#442

* Update vcpkg-tool to 2022-03-25

* Analysis of failures.

* [Most recent nightly build failed](https://dev.azure.com/vcpkg/public/_build/results?buildId=69427)
* [Validation of this tool update failed](https://dev.azure.com/vcpkg/public/_build/results?buildId=69417)

## Common to both:

PASSING, REMOVE FROM FAIL LIST: chartdir:x64-windows (.\scripts\ci.baseline.txt)
PASSING, REMOVE FROM FAIL LIST: chartdir:x64-windows-static-md (.\scripts\ci.baseline.txt)
PASSING, REMOVE FROM FAIL LIST: chartdir:x86-windows (.\scripts\ci.baseline.txt)

Probably fixed by #23701

PASSING, REMOVE FROM FAIL LIST: gmp:x64-uwp (.\scripts\ci.baseline.txt)
PASSING, REMOVE FROM FAIL LIST: gmp:x64-windows-static-md (.\scripts\ci.baseline.txt)

Probably fixed by #23466 ?

REGRESSION: colmap:x64-windows-static-md failed with BUILD_FAILED. If expected, add colmap:x64-windows-static-md=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.

I don't know exactly what changed. I observe that
* this thing depends on a *lot* of stuff
* on March 14 we didn't even attempt to build this
* the x64-windows ones are already in the baseline

so I skipped it.

REGRESSION: qtdeclarative:x64-windows. If expected, add qtdeclarative:x64-windows=fail to .\scripts\ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\1\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: qtdeclarative:x64-windows failed with BUILD_FAILED. If expected, add qtdeclarative:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.

This is a reporting change: The new world order also includes host build failures which is why it's duplicated.

See also #23714
See also #23490

I'm nervous about baslining this because it seems most of the qt world is built on top of this port

I filed #23824 about this and @Neumann-A indicated this should be fixed by #23755

REGRESSION: nettle:x64-uwp. If expected, add nettle:x64-uwp=fail to .\scripts\ci.baseline.txt.
REGRESSION: nettle:x64-windows-static-md. If expected, add nettle:x64-windows-static-md=fail to .\scripts\ci.baseline.txt.
REGRESSION: nettle:x64-uwp failed with BUILD_FAILED. If expected, add nettle:x64-uwp=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.
REGRESSION: nettle:x64-windows-static-md failed with POST_BUILD_CHECKS_FAILED. If expected, add nettle:x64-windows-static-md=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.

Didn't analyze, probably fixed by #23519 ?

REGRESSION: libgpg-error:x64-uwp. If expected, add libgpg-error:x64-uwp=fail to .\scripts\ci.baseline.txt.
REGRESSION: libgpg-error:x64-uwp failed with BUILD_FAILED. If expected, add libgpg-error:x64-uwp=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.

This was broken by VS2022 update:
```
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Microsoft\VisualStudio\v17.0\AppxPackage\Microsoft.AppXPackage.Targets(892,25): error MSB4086: A numeric comparison was attempted on "$(TargetPlatformMinVersion)" that evaluates to "" instead of a number, in condition "'$(TargetPlatformMinVersion)' >= '10.0.17200.0'". [C:\Dev\vcpkg\buildtrees\libgpg-error\x64-uwp-rel\error-1.42-2324ddbc71.clean\SMP\libgpg-error_winrt.vcxproj]
```

REGRESSION: libmikmod:x64-osx. If expected, add libmikmod:x64-osx=fail to .\scripts\ci.baseline.txt.
REGRESSION: libmikmod:x64-osx failed with BUILD_FAILED. If expected, add libmikmod:x64-osx=fail to /Users/vagrant/Data/work/2/s/scripts/azure-pipelines/../ci.baseline.txt.

Broken between [2022-03-16](https://dev.azure.com/vcpkg/public/_build/results?buildId=68947) and [2022-03-18](https://dev.azure.com/vcpkg/public/_build/results?buildId=69051). Unfortunately I don't see obvious reasons why. Nothing else depends on this and nobody has noticed in 2 weeks, so I'm baslining it for now. (Will investigate shortly...)

## Only broken in tool update:

REGRESSION: mesa:x64-windows failed with BUILD_FAILED. If expected, add mesa:x64-windows=fail to C:\a\2\s\scripts\azure-pipelines/../ci.baseline.txt.

```
-- Downloading https://gitlab.freedesktop.org/mesa/mesa/-/archive/mesa-21.2.5/mesa-mesa-21.2.5.tar.gz -> mesa-mesa-mesa-21.2.5-1.tar.gz...
-- Extracting source /Users/vagrant/Data/downloads/mesa-mesa-mesa-21.2.5-1.tar.gz
-- Applying patch swravx512-post-static-link.patch
-- Applying patch swr-msvc-2.patch
-- Applying patch swr-llvm13.patch
-- Applying patch radv-msvc-llvm13-2.patch
-- Applying patch d3d10sw.patch
-- Using source at /Users/vagrant/Data/buildtrees/mesa/src/esa-21.2.5-2df234d2b1.clean
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'mako'
CMake Error at ports/mesa/portfile.cmake:85 (message):
  Python package 'mako' needs to be installed for port 'mesa'.

  Complete list of required python packages: setuptools;mako
Call Stack (most recent call first):
  ports/mesa/portfile.cmake:91 (vcpkg_get_python_package)
  scripts/ports.cmake:145 (include)
```

Looks like this is being tracked by #23089 ; perhaps that we don't have as aggressive a recycling strategy for macos boxes as we do for the others has let different machines give different results?

## Only broken without tool update:

REGRESSION: chromium-base:x64-osx. If expected, add chromium-base:x64-osx=fail to .\scripts\ci.baseline.txt.

This one has been constantly flaky; I baselined it.

REGRESSION: libxml2:x64-osx. If expected, add libxml2:x64-osx=fail to .\scripts\ci.baseline.txt.

This port uses vcpkg_from_git and the upstream server was down during the build.

* Restore chartdir to the baseline, I thought #23732 had been merged.
@autoantwort autoantwort deleted the parse-ci.baseline.txt branch August 9, 2022 15:31
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.

5 participants