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

ci: Add feature to use different meson versions #82

Open
wants to merge 1 commit into
base: meson-upstream-master
Choose a base branch
from

Conversation

nbyavuz
Copy link
Collaborator

@nbyavuz nbyavuz commented Apr 4, 2023

We want to have a branch that uses a specific branch of meson for all tasks. Then, we can create a Cirrus cron "job" that tests Postgres againts different meson branches.

Set a different meson branch by using MESON_REPO and MESON_BRANCH environment variables. Use a bash script to install this specific meson branch and rebase current Postgres branch onto Postgres HEAD.

Fixes #81.

anarazel/pg-vm-images#70 needs to be merged first.

@nbyavuz nbyavuz requested a review from anarazel April 4, 2023 11:06
.cirrus.yml Outdated Show resolved Hide resolved
@@ -360,6 +370,9 @@ task:
CCACHE_MAXSIZE: "400M" # tests two different builds
SANITIZER_FLAGS: -fsanitize=alignment,undefined

install_meson_and_rebase_script: |
bash src/tools/ci/install_meson_and_rebase.sh ${MESON_REPO} ${MESON_BRANCH} linux

Copy link
Owner

Choose a reason for hiding this comment

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

I think it's probably worth just doing this for all tasks, rather than just this one in the matrix.

I'd really like if we could put the invocation into a single place instead of needing to repeat it in many places. But it's not exactly obvious how. Looks like I should have made all tasks in postgres' .cirrus inline the same yaml block, to make it easier to do stuff like this.

I was wondering if we could use https://cirrus-ci.org/guide/programming-tasks/ to make this easier, but I couldn't find a way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, copying same thing on multiple places doesn't seem good but I couldn't find a way too.

Copy link
Owner

Choose a reason for hiding this comment

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

I wonder, not as part of this PR, whether we should something like

task_prep: &task_prep
  # add scripts to be executed at the start of all tasks here

task_finish: &task_finish
  # add scripts to be executed at the end of all tasks here

I would like to filter the set of artifacts uploaded in case of failures, which would be a lot easier if we had something like this...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I was thinking if we could ask for a feature request for something like this to cirrus?

src/tools/ci/install_meson_and_rebase.sh Outdated Show resolved Hide resolved
src/tools/ci/install_meson_and_rebase.sh Outdated Show resolved Hide resolved
src/tools/ci/install_meson_and_rebase.sh Outdated Show resolved Hide resolved
src/tools/ci/install_meson_and_rebase.sh Outdated Show resolved Hide resolved
src/tools/ci/install_meson_and_rebase.sh Outdated Show resolved Hide resolved
@nbyavuz nbyavuz force-pushed the meson-test-all-versions branch 3 times, most recently from 9bf0618 to 5c986e6 Compare June 23, 2023 14:12
@nbyavuz
Copy link
Collaborator Author

nbyavuz commented Jun 23, 2023

I pushed this with a commit to run on 'pg-ci-images-dev' project since anarazel/pg-vm-images#70 didn't run on 'pg-ci-images' yet. After anarazel/pg-vm-images#70 is merged, I will push this without "run on 'pg-ci-images-dev'" commit.

@nbyavuz nbyavuz requested a review from anarazel June 23, 2023 14:23
@nbyavuz nbyavuz force-pushed the meson-test-all-versions branch 2 times, most recently from f116dd3 to 4d0d3d7 Compare June 23, 2023 19:12
@anarazel
Copy link
Owner

I just tested it for the new meson version about to be released - very useful!

I got one failure that i'm somewhat confused by: https://cirrus-ci.com/task/6190199437262848?logs=configure#L8
that doesn't seem related to the new meson version, but doesn't happen otherwise?

An improvement for this would be to list which commit we rebased onto:
https://cirrus-ci.com/task/6190199437262848?logs=install_meson_and_rebase#L46
doesn't allow to figure that out:

[21:31:59.899] + git remote add default-postgres https://github.com/postgres/postgres.git
[21:31:59.930] + git fetch default-postgres master
[21:32:04.399] From https://github.com/postgres/postgres
[21:32:04.399]  * branch              master     -> FETCH_HEAD
[21:32:04.399]  * [new branch]        master     -> default-postgres/master
[21:32:04.415] + git rebase --no-verify default-postgres/master
[21:32:05.879] Rebasing (1/1)
Successfully rebased and updated detached HEAD.

Something like

echo Rebasing onto: $(git show --no-patch --abbrev-commit  --pretty=oneline default-postgres/master)

maybe?

@nbyavuz nbyavuz force-pushed the meson-test-all-versions branch 2 times, most recently from dfc33cf to b9d7baf Compare July 12, 2023 12:50
@nbyavuz
Copy link
Collaborator Author

nbyavuz commented Jul 12, 2023

I got one failure that i'm somewhat confused by: https://cirrus-ci.com/task/6190199437262848?logs=configure#L8 that doesn't seem related to the new meson version, but doesn't happen otherwise?

I found the cause and fixed it. It seems rebase part changing file endings to CLRF on Windows, I added git config core.autocrlf false.

Also, I realized that CI only collects *.diffs files but sometimes diff logs are written in *.diff files (for example pg_bsd_indent). It can be fixed by editing on_failure parts but I couldn't decide between editing on_failure parts and changing *.diff to *.diffs.

Something like

echo Rebasing onto: $(git show --no-patch --abbrev-commit  --pretty=oneline default-postgres/master)

Added.

@anarazel
Copy link
Owner

I got one failure that i'm somewhat confused by: https://cirrus-ci.com/task/6190199437262848?logs=configure#L8 that doesn't seem related to the new meson version, but doesn't happen otherwise?

I found the cause and fixed it. It seems rebase part changing file endings to CLRF on Windows, I added git config core.autocrlf false.

Great.

Also, I realized that CI only collects *.diffs files but sometimes diff logs are written in *.diff files (for example pg_bsd_indent). It can be fixed by editing on_failure parts but I couldn't decide between editing on_failure parts and changing *.diff to *.diffs.

Heh, yea, I noticed that too and even wrote an email about it:
https://www.postgresql.org/message-id/20230711233307.hu4wetabjm5f7ver%40awork3.anarazel.de
and am planning to rename the file.

Something like

echo Rebasing onto: $(git show --no-patch --abbrev-commit  --pretty=oneline default-postgres/master)

Added.

Thanks!

# Rebase current branch onto Postgres HEAD
rebase_onto_postgres () {
echo "Rebasing onto: $(git show --no-patch --abbrev-commit --pretty=oneline default-postgres/master)"
# freebsd and linux run commands as postgres user
Copy link
Owner

Choose a reason for hiding this comment

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

Is that actually required? It's true that they run configure / build as postgres, but that shouldn't be affected by source code permissions?

git config core.filemode false
# windows changes file endings to crlf, causing test failures
git config core.autocrlf false
git reset --hard
Copy link
Owner

Choose a reason for hiding this comment

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

Why is the reset --hard needed?

esac

# After repartition, hidden files are not copied. Copy them to
# working dir
Copy link
Owner

Choose a reason for hiding this comment

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

I'd rather fix that in the repartition step?

PYTHON=python
;;
*)
PYTHON=python3
Copy link
Owner

Choose a reason for hiding this comment

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

I assume there's no 'python3' on windows? If not, perhaps we should just add an alias to the image? That's annoying in other places too...

@@ -360,6 +370,9 @@ task:
CCACHE_MAXSIZE: "400M" # tests two different builds
SANITIZER_FLAGS: -fsanitize=alignment,undefined

install_meson_and_rebase_script: |
bash src/tools/ci/install_meson_and_rebase.sh ${MESON_REPO} ${MESON_BRANCH} linux

Copy link
Owner

Choose a reason for hiding this comment

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

I wonder, not as part of this PR, whether we should something like

task_prep: &task_prep
  # add scripts to be executed at the start of all tasks here

task_finish: &task_finish
  # add scripts to be executed at the end of all tasks here

I would like to filter the set of artifacts uploaded in case of failures, which would be a lot easier if we had something like this...

We want to have a branch that uses a specific branch of meson for all
tasks. Then, we can create a Cirrus cron "job" that tests Postgres
againts different meson branches.

Set a different meson branch by using `MESON_REPO` and
`MESON_BRANCH` environment variables. Use a bash script to install
this specific meson branch and rebase current Postgres branch onto
Postgres HEAD.

Fixes anarazel#81.
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.

None yet

3 participants