Skip to content
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

updated buildkite pipeline generation #65574

Merged
merged 1 commit into from
Sep 11, 2023
Merged

updated buildkite pipeline generation #65574

merged 1 commit into from
Sep 11, 2023

Conversation

metaflow
Copy link
Contributor

@metaflow metaflow commented Sep 7, 2023

  • fixed build for linux (clang was missing)

  • removed /monolithic-.. from build directory - it does not add anything and makes path longer for windows which is not great;

  • added env-based configuration to control cache and agent targeting;

  • print (s)ccache stats to file not to pullute normal log.

@metaflow metaflow added the infrastructure Bugs about LLVM infrastructure label Sep 7, 2023
@metaflow metaflow changed the title Scheduled bildkite pipeline generation Scheduled buildkite pipeline generation Sep 7, 2023
Copy link
Contributor

@aeubanks aeubanks left a comment

Choose a reason for hiding this comment

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

the commit title doesn't make sense, can you make it something like "Various fixes to buildkite pipeline generation"

rm -rf ${BUILD_DIR}

ccache --zero-stats
ccache --show-config

if [[ -n "${CLEAR_CACHE:-}" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

how can you set this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you can do that when creating a build in buildkite UI or triggering through API with "environment" varaibles. It's not available for "normal" users

- 'C:\\BuildTools\\Common7\\Tools\\VsDevCmd.bat -arch=amd64 -host_arch=amd64'
- 'bash .ci/monolithic-windows.sh "clang-tools-extra;flang;libclc;lld;llvm;mlir;polly;pstl" "check-all"'
- C:\\BuildTools\\Common7\\Tools\\VsDevCmd.bat -arch=amd64 -host_arch=amd64
- bash .ci/monolithic-windows.sh "clang-tools-extra;flang;libclc;lld;llvm;mlir;polly;pstl;clang" "check-all"
Copy link
Contributor

Choose a reason for hiding this comment

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

make alphabetical just to be consistent and look nice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

agents:
queue: 'linux'
- artifacts/**/*
- '*_result.json'
Copy link
Contributor

Choose a reason for hiding this comment

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

quoting is inconsistent, I believe all of these don't need quotes
(other files as well)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

- fixed build for linux (clang was missing)

- removed /monolithic-.. from build directory - it does not add anything
  and makes path longer for windows which is not great;

- added env-based configuration to control cache and agent targeting;

- print (s)ccache stats to file not to pullute normal log.
@metaflow metaflow changed the title Scheduled buildkite pipeline generation updated buildkite pipeline generation Sep 11, 2023
@metaflow
Copy link
Contributor Author

@metaflow metaflow merged commit 2484678 into llvm:main Sep 11, 2023
2 checks passed
@metaflow metaflow deleted the ci-gen branch September 11, 2023 11:20
@joker-eph
Copy link
Collaborator

This seems broken: https://buildkite.com/llvm-project/github-pull-requests/builds/1350#018a8848-f70e-47f4-8aec-0bb48c49a7ca

$ trap 'kill -- $' INT TERM QUIT; .ci/generate-buildkite-pipeline-premerge \| buildkite-agent pipeline upload
  | 2023-09-12 07:26:50 INFO   Reading pipeline config from STDIN
  | 2023-09-12 07:26:50 INFO   Updating BUILDKITE_COMMIT to "6de56a7dc4ff114eb36f42f1dbe8c63e557f061b"
  | Files modified:
  | mlir/include/mlir/Dialect/Affine/IR/AffineOps.h
  | mlir/include/mlir/Dialect/Affine/IR/AffineOps.td
  | mlir/include/mlir/IR/SymbolInterfaces.td
  | mlir/lib/Dialect/Affine/IR/AffineOps.cpp
  | mlir/test/Dialect/Affine/invalid.mlir
  | mlir/test/Dialect/Affine/load-store-invalid.mlir
  | Directories modified:
  | mlir
  | 2023-09-12 07:26:50 FATAL  Pipeline parsing of "(stdin)" failed: line 5: did not find expected alphabetic or numeric character
  | 🚨 Error: The command exited with status 1

This seems to match the - '*_result.json' replaced with - *_result.json

@metaflow
Copy link
Contributor Author

metaflow commented Sep 12, 2023

thanks for the heads up @joker-eph ! That should be fixed now but you need to rebase your PR to see the change. I guess noop commit update should do

@metaflow
Copy link
Contributor Author

ah you've merged it already 😄

@ldionne
Copy link
Member

ldionne commented Sep 12, 2023

@metaflow The reason for not adding clang to the monolithic pipeline is that clang now has its own pipeline like libc++. After your patch we will be building and testing clang twice for no reason. Can you remove it again?

@metaflow
Copy link
Contributor Author

@ldionne w/o clang flang will not compile. Do you also check flang and all dependencies of clang in clang pipeline?

@ldionne
Copy link
Member

ldionne commented Sep 12, 2023

I think it's fine to add clang to the set of projects in LLVM_ENABLE_PROJECTS for dependency purposes, but we could avoid calling check-clang. I guess it's moot cause we're using check-all right now.

We don't check flang in the Clang pipeline because the Clang pipeline's primary responsibility should be to test Clang, not other downstream projects.

@joker-eph
Copy link
Collaborator

I think it's fine to add clang to the set of projects in LLVM_ENABLE_PROJECTS for dependency purposes, but we could avoid calling check-clang. I guess it's moot cause we're using check-all right now.

FYI, the script for premerge generates invocations like:

'./.ci/monolithic-linux.sh "clang;flang;llvm;mlir" "check-flang check-mlir"'

So even though clang is enabled, it shouldn't be tested I think? (we don't want to test LLVM either when it isn't touched)

@ldionne
Copy link
Member

ldionne commented Sep 12, 2023

The scheduled build script does call check-all, but you're right about the script that generates pre-commit CI pipelines.

ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
- fixed build for linux (clang was missing)

- removed /monolithic-.. from build directory - it does not add anything
and makes path longer for windows which is not great;

- added env-based configuration to control cache and agent targeting;

- print (s)ccache stats to file not to pullute normal log.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Bugs about LLVM infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants