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: Auto-determine checks to run #106

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

ci: Auto-determine checks to run #106

wants to merge 3 commits into from

Conversation

jtojnar
Copy link
Member

@jtojnar jtojnar commented May 11, 2022

This will allow us to support multiple platforms easily.

Still some issues:

Supersedes: #85

@jtojnar jtojnar force-pushed the ci-cleanup branch 12 times, most recently from d188077 to 5a07c59 Compare May 11, 2022 15:03
@drupol
Copy link
Collaborator

drupol commented May 11, 2022

Hi !

There's a couple of things that confuses me.

  1. The file .github/workflows/test.yaml, while ending with .yaml, it is a json file.
  2. Seeing a .nix file in the .github/workflow/ directory where it is supposed to only contains Github workflows.

@jtojnar
Copy link
Member Author

jtojnar commented May 11, 2022

YAML is sorta superset of JSON. And GitHub only supports ya?ml as extension.

  • Seeing a .nix file in the .github/workflow/ directory where it is supposed to only contains Github workflows.

It is a helper script for generating the workflow. GitHub will ignore it.

@jtojnar jtojnar marked this pull request as draft May 11, 2022 22:47
drupol
drupol previously approved these changes May 15, 2022
Copy link
Collaborator

@drupol drupol left a comment

Choose a reason for hiding this comment

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

👍

@drupol
Copy link
Collaborator

drupol commented May 15, 2022

Maybe this needs a rebase?

@drupol
Copy link
Collaborator

drupol commented May 15, 2022

Just did it.

Comment on lines +20 to +25
- name: Generate actions
run: nix-shell .github/workflows/workflow-from-flake-checks.nix
test:
needs:
- prepare-ci
uses: ./.github/workflows/test.yaml
secrets: inherit
Copy link
Member Author

Choose a reason for hiding this comment

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

This does not do anything. If we decide to go the way of generated CI file, we should probably drop this file, or replace it with a check that the generated workflow is in sync with the checks.

description = "Build Xdebug extension";
drv = { php, ... }: php.extensions.xdebug;
};
{
Copy link
Member Author

Choose a reason for hiding this comment

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

This probably needs to be a list so that we can have ordering.

};
}
]
++ lib.mapAttrsToList
Copy link
Member Author

Choose a reason for hiding this comment

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

Or maybe it would be enough to add sort here, that would ensure php test is first (not sure if there are any other dependencies – if yes, we should probably express them in checks.nix and use toposort).

Exposing passhtru attributes as the derivation attributes is done by `stdenv.mkDerivation` so if we just add them to the passthru attribute of the resulting derivation attrset, they will be only available inside the passthru attribute. We can just append those attributes to the derivation attrset directly, it won’t affect the derivation either way.
This will basically allow us to treat `checks.nix` like something that supports having its dependencies injected by `callPackages`, allowing us to simplify the expression a bit. It will also allow other scripts to get the check names trivially.
This will allow us to support multiple platforms easily.
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.

2 participants