Horton
Menu

Re: Adding CI to our tree

Melanie Plageman <melanieplageman@gmail.com>

View thread
From: Melanie Plageman <melanieplageman@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>, Thomas Munro <thomas.munro@gmail.com>, Andrew Dunstan <andrew@dunslane.net>
Date: 2021-11-19T22:17:44Z
Lists: pgsql-hackers

Attachments

Hi,

I reviewed the first patch in this set
(v2-0001-ci-Add-CI-for-FreeBSD-Linux-MacOS-and-Windows-uti.patch).

For the README, I found the instructions very clear. My only concern is
that the cirrus-ci UI will change and the instructions on how to enable
cirrus-ci on a repository will not be accessible in the same way in the
future.
That being said, I found your instructions easier to follow than those
on [1].
Perhaps it is better to wait until it becomes a problem and then, at
that point, change the README to guide people to the quickstart link.

I have attached a patch which does a small refactor using a yaml anchor
and aliases (tried it and it seems to work for me).

A few questions and thoughts:

- do you not need to change the default core resource limits for
  FreeBSD?

- Would you find it valuable to set a few more coredump_filter bits?
  Might be worth setting bits 2 and 3 (see [2])-- in addition to the
  defaults (on Linux -- I don't know what the equivalent is on other
  platforms).

- I found this line a bit confusing, so maybe it is worth a comment
  sysinfo_script:
    - export || true

- For the docker files, I think it is recommended to run "docker build"
  only from within the specific build context (not in the top-level
  directory), so I don't think you should need the dockerignore file.

  Also, instead of putting the two docker files in the same directory,
  you could put them in dedicated directories (making those directories
  their build context). That way if you change one you don't end up
  rebuilding the other.

- In ci/docker/linux_debian_bullseye, you can make this change:
  -  apt-get clean
  +  apt-get clean && \
  +  rm -f /var/lib/apt/lists/*

  to make that layer smaller.

- Melanie

[1] https://cirrus-ci.org/guide/quick-start/
[2] https://man7.org/linux/man-pages/man5/core.5.html

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. ci: enable zstd where available.

  2. ci: compile with -Og where applicable.

  3. ci: include hints how to install OS packages.

  4. ci: fix copy-paste mistake in 16eb8231d1b.

  5. ci: macos: align sysinfo_script to other tasks.

  6. ci: Only use one artifact instruction for logs.

  7. ci: s/CCACHE_SIZE/CCACHE_MAXSIZE/.

  8. pg_basebackup: Skip a few more fsyncs if --no-sync is specified.

  9. TAP tests: check for postmaster.pid anyway when "pg_ctl start" fails.

  10. Don't enable fsync in src/test/recovery/t/008_fsm_truncation.pl.

  11. ci: windows: run initdb with --no-sync.

  12. ci: windows: enable build summary to make it easier to spot warnings / errors.

  13. ci: Add continuous integration for github repositories via cirrus-ci.

  14. Fix TestLib::slurp_file() with offset on windows.