-
-
Notifications
You must be signed in to change notification settings - Fork 818
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
dev/core#5611 Exclude tests and tools directories from exports #31498
base: master
Are you sure you want to change the base?
Conversation
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
The issue associated with the Pull Request can be viewed at https://lab.civicrm.org/dev/core/-/issues/5611 |
I tried this out. When using I tested by composer init'ing a little project and then adding a vcs repo, like below, and then "minimum-stability": "dev",
"repositories": [
{
"type": "vcs",
"url": "https://github.com/colemanw/civicrm-core"
}
], |
e66cede
to
84e68c8
Compare
@demeritcowboy I've prepended them with a wildcard. See if that works. |
84e68c8
to
fb68d1a
Compare
Looks better. I'll put merge-ready because I don't know if there's a companion update needed for drupal 8+ tests, and I'm not sure where they run nowadays - I took a quick look at test.civicrm.org but it wasn't obvious. |
I added the run-drupal-x tag - so I think it does need a corresponding buildkit or jenkins update since I see for example: |
Yeah, at the end of MM discussion, I got a bit nervous about that extra load from getting all sources. FWIW, the relevant files for test build are https://github.com/civicrm/civicrm-buildkit/blob/master/app/config/drupal10-dev/download.sh and https://github.com/civicrm/civicrm-buildkit/blob/master/app/config/drupal9-dev/download.sh In playing with that on my local, I see differences in both run-time and disk-usage (source: 58s, 2132mb; dist: 35s, 894mb). For core/d10 test-jobs, I expect those stats should be representative of an average test-build. However, given the use of tmpfs and per-runner caches, the increased footprint would be quite visible (but not fatal). Thinking outloud, another consideration of the extra work is the reliability of the operations:
Anyway, where I'm heading is that CI jobs probably shouldn't use broad options ( https://stackoverflow.com/a/56602818/4195300 I'm gonna play with adding that to civibuild... |
Arg. This feels kind of like a composer bug/quirk to me. The distinctive feature of "repositories": {
"civicrm-packages": {
"type": "path",
"url": "./src/civicrm-packages",
"options": {
"symlink": ...
}
},
"civicrm-drupal-8": {
"type": "path",
"url": "./src/civicrm-drupal-8",
"options": {
"symlink": ...
}
},
"civicrm-core": {
"type": "path",
"url": "./src/civicrm-core",
"options": {
"symlink": ...
}
},
"0": {
"type": "composer",
"url": "https://packages.drupal.org/8"
}
}, (Why: This helps with testing proposed revisions in The application of
A possible work-around is to enable symlink mode (civicrm/civicrm-buildkit#891). That nominally provides the files that we want, but then |
Overview
Avoid distributing test files in packaged downloads.
See https://lab.civicrm.org/dev/core/-/issues/5611
Technical Details
I'm not sure if this only fixes composer-based installs and we also need to do something for our packaging script to avoid putting
ext/*/tests
directories in the zip/tar files.