| [02:20:27] | * SebastianBui[m] has joined #farmos |
| [02:29:08] | <SebastianBui[m]> | hi just wanted to introduce myself as a newbie to the farmos. i am not technically inclined but am here to learn mostly. hope thats okay. i wont have much technical knowledge to contribute. hope that does not disqualify me from being here. |
| [04:58:17] | * farmBOT has joined #farmos |
| [07:08:22] | <mstenta[m]> | Hi Sebastian Bui - welcome! You're in the right place, regardless of your background. :-) |
| [07:08:57] | <mstenta[m]> | I recommend joining the community forum too (better than chat for discussions): https://farmOS.discourse.group |
| [07:09:04] | <mstenta[m]> | Hope to see you around! |
| [10:55:09] | <symbioquine[m]> | Created https://github.com/farmOS/farmOS/pull/864 as a draft. Still need to test... |
| [10:57:03] | <mstenta[m]> | Wow awesome! |
| [10:58:20] | <paul121[m]> | symbioquine: so this doesn't require any changes to the dockerfile? would upcoming/remaining changes for the dockerfile from the previous PR potentially break multi-platform builds? |
| [10:59:03] | <paul121[m]> | I've been wanting to pickup those remaining changes but it seemed like these buildx discussions were going to conflict so I've been waiting... |
| [11:00:23] | <symbioquine[m]> | paul121[m]: I don't think so - obviously anything can break anything but... |
| [11:01:15] | <mstenta[m]> | It would be awesome to offer v3.3.0 on ARM! |
| [11:01:36] | <mstenta[m]> | (added this PR to the milestone, but no pressure if it doesn't make it in in time) |
| [11:01:55] | <mstenta[m]> | (just thinking that would be another cool thing to mention in the minor release blog post!) |
| [11:02:25] | <mstenta[m]> | (there's also a fair bit to do before v3.3.0 is ready anyway...) |
| [11:03:18] | <symbioquine[m]> | I guess I should look over those additional proposed Docker file changes again in order to answer your question more confidently @paul121 |
| [11:05:44] | <paul121[m]> | symbioquine[m]: This comment is a good tldr of what I'm thinking: https://github.com/farmOS/farmOS/pull/751#pullrequestreview-2186757356 |
| [11:10:18] | <mstenta[m]> | re: the composer ENV vars... my preference is to only include ones that are demonstrably necessary |
| [11:11:09] | <mstenta[m]> | specifically COMPOSER_ALLOW_SUPERUSER and COMPOSER_NO_INTERACTION |
| [11:11:29] | <mstenta[m]> | (we already set COMPOSER_MEMORY_LIMIT, unless you're talking about changing that somehow) |
| [11:15:07] | <paul121[m]> | It looks like COMPOSER_ALLOW_SUPERUSER and COMPOSER_MEMORY_LIMIT are already set. Maybe we got confused because the PR was just moving those within the Dockerfile |
| [11:23:13] | <symbioquine[m]> | <paul121[m]> "This comment is a good tldr of..." <- > <@paul121:matrix.org> > <@symbioquine:matrix.org> I guess I should look over those additional proposed Docker file changes again in order to answer your question more confidently @paul121... (full message at <https://matrix.org/_matrix/media/v3/download/matrix.org/akYvJUqTddeRnzZz...) |
| [11:30:32] | <mstenta[m]> | Ah maybe. I knew COMPOSER_MEMORY_LIMIT was already there... |
| [11:31:16] | <mstenta[m]> | I think the bit I'm most interested in is around disolving build-farmOS.sh |
| [11:32:51] | <mstenta[m]> | but all in all, it seems like everything is just cleanup/rearranging at this point. we might make things a little smaller/more efficient here and there, but i don't think there are any big changes |
| [11:33:51] | <mstenta[m]> | https://github.com/farmOS/farmOS/pull/855 was the bigger change I think |
| [11:34:55] | <mstenta[m]> | before #855 we were essentially running composer install twice to build the dev image (once in the production image build, and then we ran it again to overwrite that in the dev image. now we essentially "fork" the build into doing one or the other. |
| [11:35:45] | <mstenta[m]> | (i guess in terms of deliver.yml we're still running composer install the same number of times, technically speaking... but now it's possible to build a standalone dev image with a single composer install step) |
| [11:36:29] | <mstenta[m]> | makes me wonder... would we be able to squeeze a little more speed out of the build steps if we built the prod and dev images in parallel now 🤔 |
| [11:36:42] | <mstenta[m]> | probably splitting hairs... i doubt it would be a big improvement |
| [11:41:09] | <paul121[m]> | mstenta[m]: do we only need to build prod when tagging a release? |
| [11:41:46] | <mstenta[m]> | yep i think so! that could be another improvement |
| [11:42:08] | <mstenta[m]> | 3.x is always pushed in parallel, which would build the dev image at the same commit |
| [11:42:37] | <symbioquine[m]> | Hmmm, I'm not convinced... |
| [11:42:59] | <mstenta[m]> | oh wait |
| [11:43:02] | <mstenta[m]> | sorry i misunderstood |
| [11:43:15] | <symbioquine[m]> | I think we run the tests against the Dev IMG as a prerequisite to tagging a release. |
| [11:43:19] | <mstenta[m]> | no, it's still useful to have a farmos/farmos:3.x image that does not include dev dependencies etc |
| [11:44:44] | <mstenta[m]> | paul121: or did you mean: "can we SKIP building dev when tagging a release?" that's how I interpretted it initially, but maybe you meant "can we SKIP building prod when we're only pushing 3.x" |
| [11:45:07] | <mstenta[m]> | skipping dev build on tag push could make sense |
| [11:45:25] | <mstenta[m]> | we don't publish farmos/farmos:x.y.z-dev |
| [11:45:47] | <mstenta[m]> | that would certainly shave off some time for tagged releases |
| [11:46:12] | <paul121[m]> | oh - yeah I meant skip building prod unless we are making a release, but it makes sense we want the 3.x image. |
| [11:46:13] | <mstenta[m]> | > I think we run the tests against the Dev IMG as a prerequisite to tagging a release. |
| [11:46:13] | <mstenta[m]> | Oh but that's the critical bit symbioquine is right :-) |
| [11:46:37] | <mstenta[m]> | yep yep... so everything makes sense as-is, in both cases :-) |
| [11:48:52] | <paul121[m]> | it feels like we should be "testing the prod image" since that it is what might be used in the wild... and it is built separately from testing... |
| [11:49:20] | <paul121[m]> | s/testing/*dev/ |
| [11:49:58] | <paul121[m]> | the only difference would be a separate composer install run. in theory they will be the same, but it is composer... :D |
| [11:50:02] | <mstenta[m]> | dev dependencies are installed with prod though |
| [11:50:10] | <mstenta[m]> | aren't* i mean |
| [11:50:43] | <mstenta[m]> | (eg: phpunit) |
| [11:50:54] | <paul121[m]> | it would be nice if we could share a composer lock file but only install dev dependencies into the dev image |
| [11:51:16] | <mstenta[m]> | that's essentially what we do... just without committing the composer.lock file |
| [11:51:38] | <mstenta[m]> | we run composer install --no-dev for prod, and composer install for dev |
| [11:52:05] | <mstenta[m]> | (whether or not we commit composer.lock is another question... but wouldn't change how that works) |
| [11:54:08] | <paul121[m]> | no, its not about committing the lock file. we run composer install two separate times "from scratch", so I don't think we can guarantee they would end up with the same lock file/dependencies |
| [11:54:59] | <mstenta[m]> | that's true. that's the question about committing composer.lock. |
| [11:55:12] | <mstenta[m]> | but we would be running those same commands regardless |
| [11:55:15] | <symbioquine[m]> | It wouldn't need to be committed to git necessarily |
| [11:55:22] | <symbioquine[m]> | Just shared between the two stages I think |
| [11:55:23] | <mstenta[m]> | true |
| [11:55:35] | <mstenta[m]> | that would be a nice next step perhaps |
| [11:55:41] | <mstenta[m]> | for that specific concern |
| [11:55:55] | <symbioquine[m]> | We could have a "composer lock resolver" stage that just does that, then uses the lock file in either of the target stages. |
| [11:56:19] | <symbioquine[m]> | Then in my proposed GH workflow, that stage would be cached and shared. |
| [11:56:25] | <symbioquine[m]> | (I think) |
| [11:56:36] | <mstenta[m]> | notably, we're talking about a composer.lock at the project level, right? not the farmos/farmos install profile |
| [11:56:38] | <mstenta[m]> | ? |
| [11:56:52] | <mstenta[m]> | this: https://github.com/farmOS/composer-project |
| [11:56:52] | <paul121[m]> | yep |
| [11:57:14] | <symbioquine[m]> | mstenta[m]: Well, derived from that I think. |
| [11:57:48] | <mstenta[m]> | yea. we basically just git clone that, and a temporary composer.lock gets generated in the build environments, but is never committed |
| [11:58:04] | <mstenta[m]> | but you're right paul121 - that's getting created twice, and *could* be different |
| [11:58:25] | <symbioquine[m]> | mstenta[m]: But for example [this line](https://github.com/farmOS/farmOS/blob/3a73ccf6360ba309ee657e2d645d29c239...) alters the `composer.json` prior to running `composer install` right? |
| [11:58:54] | <mstenta[m]> | true. we do make some changes to the composer.json after cloning it |
| [11:59:02] | <mstenta[m]> | that's also how we point it to another repo for PRs |
| [11:59:15] | <mstenta[m]> | https://github.com/farmOS/farmOS/blob/3a73ccf6360ba309ee657e2d645d29c239... |
| [11:59:35] | <mstenta[m]> | and https://github.com/farmOS/farmOS/blob/3a73ccf6360ba309ee657e2d645d29c239... |
| [12:00:35] | <paul121[m]> | If we check the production image I think it will actually have dev dependencies in it's lock file - which is fine - but also confirms a prerequisite to do a separate "composer lock resolver" step. It means that composer install --no-dev should respect not including the dev dependencies even if present in a lock file :-) |
| [12:00:48] | <mstenta[m]> | this is a side track but... why does it take so long to push/pull docker images to/from github cache?? |
| [12:01:23] | <paul121[m]> | Maybe relevant: https://github.com/farmOS/farmOS/pull/764 |
| [12:02:04] | <mstenta[m]> | Oh yea that PR that wotnak started might overlap with what you're doing too symbioquine |
| [12:02:46] | <symbioquine[m]> | mstenta[m]: Yeah, I wasn't aware of that one... |
| [12:03:35] | <mstenta[m]> | It was built on top of almavizca's original PR, so needed to be rerolled, but I didn't look closely at it |
| [12:04:30] | <mstenta[m]> | paul121: yea afaik there isn't a way to JUST build a `composer.lock` file, you need to run `composer install`... so this might be moot |
| [12:04:57] | <mstenta[m]> | https://stackoverflow.com/questions/44805083/can-composer-generate-the-c... |
| [12:05:55] | <symbioquine[m]> | https://stackoverflow.com/a/70972005 |
| [12:05:56] | <mstenta[m]> | this is a bit tricky... (if i'm thinking about it correctly) |
| [12:06:10] | <paul121[m]> | mstenta[m]: No, I believe we do that with `composer require farmos/farmos:${FARMOS_COMPOSER_VERSION} --no-install` |
| [12:06:10] | <mstenta[m]> | because we are trying to balance two things... |
| [12:06:49] | <mstenta[m]> | hmm i'm not sure that makes composer.lock 🤔 worth testing! |
| [12:07:01] | <mstenta[m]> | it might only update composer.json |
| [12:07:52] | <mstenta[m]> | the two things we are trying to balance are: 1) building a single composer.lock (to ensure that the same is used in both prod and dev builds), and 2) only running composer install once per dev/prod build |
| [12:08:06] | <symbioquine[m]> | ACTION uploaded an image: (97KiB) < https://matrix.org/_matrix/media/v3/download/matrix.org/cGLhNLzaTKgFEoTm... > |
| [12:08:09] | <symbioquine[m]> | https://getcomposer.org/doc/03-cli.md#update-u-upgrade |
| [12:09:06] | <mstenta[m]> | > https://stackoverflow.com/a/70972005 |
| [12:09:06] | <mstenta[m]> | symbioquine: According to this, it sounds like you can't run `composer update --no-install` to generate a `composer.lock` though |
| [12:09:14] | <symbioquine[m]> | I think so |
| [12:09:26] | <mstenta[m]> | You need a composer.lock to begin with (I haven't tested... just going by that answer) |
| [12:09:42] | <symbioquine[m]> | Anyway, gotta run. Hopefully I'll get a chance to look into this more from my end tomorrow morning... |
| [12:10:01] | <paul121[m]> | Yea, I actually do this a lot in my own build/deploy process. When I update farmos I do something like composer update farmos/farmos --no-install and it updates the lock file. Then I commit composer.lock and push to my deploy process and it installs the new deps in an environment there. |
| [12:10:15] | <mstenta[m]> | Right. But you have a lock file already in that case, right? |
| [12:10:18] | <mstenta[m]> | I do that too. |
| [12:10:28] | <mstenta[m]> | But in this case we don't have a lock file. |
| [12:11:01] | <mstenta[m]> | I think the lock file only gets created on composer install - but i could be wrong! |
| [12:11:16] | <paul121[m]> | I believe it does but will test again :-) |
| [12:12:42] | <mstenta[m]> | I'm curious now too :-) |
| [12:12:54] | <mstenta[m]> | I'll try a clean test from scratch |
| [12:15:45] | <mstenta[m]> | ok interesting... |
| [12:15:54] | <mstenta[m]> | # composer update farmos/farmos --no-install |
| [12:15:54] | <mstenta[m]> | Cannot update only a partial set of packages without a lock file present. Run `composer update` to generate a lock file. |
| [12:16:49] | <mstenta[m]> | But then when I run composer update it appears to do the same exact thing as composer install... |
| [12:17:04] | <mstenta[m]> | It makes composer.lock but also installs everything (vendor, web etc are all created) |
| [12:17:16] | <mstenta[m]> | So composer update without composer.lock == composer install |
| [12:18:36] | <mstenta[m]> | So... I don't think we can share a composer.lock file between dev and prod builds without taking a step backwards in the sense that we need to build one first )without --no-dev) and then build again on top of that (with --no-dev) |
| [12:18:50] | <mstenta[m]> | Orrrr.... unless we start committing composer.lock ;-) |
| [12:18:55] | <paul121[m]> | <paul121[m]> "No, I believe we do that with `..." <- try `composer require` |
| [12:19:09] | <mstenta[m]> | K let me reset |
| [12:20:30] | <mstenta[m]> | oh looks like that worked! |
| [12:20:42] | <mstenta[m]> | much faster :-) and only the composer.lock file was created |
| [12:21:16] | <mstenta[m]> | oh that's great! (someone should update that SO answer 😆) |
| [12:21:23] | <paul121[m]> | This might be an artifact of one of the composer plugins... maybe merge plugin? That runs something "twice".... |
| [12:21:57] | <mstenta[m]> | oh hmmmmmmm good question... i wonder if this misses anything |
| [12:23:03] | <mstenta[m]> | gah that's probably an issue... because i think merge plugin looks at stuff in the vendor directory to find things that need to be merged |
| [12:23:09] | <mstenta[m]> | (i think) |
| [12:26:54] | <mstenta[m]> | i don't know if there's any way around it... i think if we want to ensure that composer.lock is the same for prod and dev builds we would NEED to run composer install to build the dev codebase and run composer install --no-dev AGAIN from th same composer.lock that was generated to build the prod codebase |
| [12:27:08] | <mstenta[m]> | Which essentially undoes the improvement that our recent PR just made |
| [12:27:40] | <mstenta[m]> | But maybe that's just something we have to accept |
| [12:29:22] | <mstenta[m]> | (perhaps ultimately because we use composer merge, we're stuck with this ugly truth) |
| [12:30:26] | <mstenta[m]> | (i am testing to confirm this now) |
| [12:30:28] | <paul121[m]> | Is there an issue with doing the install twice? The second time should be much faster since all the dependencies are resolved |
| [12:31:08] | <paul121[m]> | The first install would be in common, the prod could do composer install --no-dev to remove dev dependencies |
| [12:31:18] | <mstenta[m]> | No not really. It's what we were doing before the recent PR. |
| [12:31:29] | <mstenta[m]> | yup |
| [12:31:40] | <mstenta[m]> | I think that's the only way to ensure we share composer.lock |
| [12:32:20] | <mstenta[m]> | (this would still be an improvement over what we had before the recent PR though... because previously we deleted /var/farmOS and ran composer install again from scratch) |
| [12:34:25] | <mstenta[m]> | > (i am testing to confirm this now) |
| [12:34:26] | <mstenta[m]> | confirmed. the `composer.lock` that is generated from `composer require farmos/farmos:3.2.3 --no-install` is different from the `composer.lock` generated from `composer install`. at quick glance it looks like it's missing everything that would be added by composer merge. |
| [12:34:57] | <mstenta[m]> | iirc composer merge works by essentially running composer update twice itself when composer install runs... or something like that |
| [12:35:48] | <mstenta[m]> | anyway... i need to work on other stuff. this was an interesting dig though. :-) |
| [12:36:27] | <mstenta[m]> | sounds like we have one way forward... a composer install build step before both dev/prod build steps, and then another composer install --no-dev for prod |
| [12:37:20] | <mstenta[m]> | what does this mean for symbioquine's `buildx` work though... will we be building multiple `composer.lock`s anyway? (one for each architecture) |
| [12:37:43] | <mstenta[m]> | (assuming that we use a matrix to run tests on all archs too) |
| [12:39:08] | <paul121[m]> | ah hmm. yeah I guess so |
| [12:39:41] | <mstenta[m]> | unless we did one arch first, built the composer.lock, and then copied it into the builds for other archs |
| [12:39:59] | <mstenta[m]> | feels like it's starting to spiral out 😅 |
| [12:40:47] | <mstenta[m]> | or.... maybe we don't need to run composer install inside the docker container? 🤔 |
| [12:41:20] | <mstenta[m]> | what if we had a simple step before all the docker builds that just ran composer install in the github action native environment (not in a container within that) |
| [12:41:30] | <mstenta[m]> | presumably composer will work the same regardless |
| [12:42:01] | <mstenta[m]> | (just thinking out loud, not suggesting) |
| [12:42:56] | <paul121[m]> | yea... and I guess ultimately, there are various shortcomings in using this single prod github image. Most people will want it further customized with additional contrib dependencies. In this regard the state of the composer.lock is somewhat out of our control |
| [12:43:37] | <paul121[m]> | Honestly it might make more sense to do the buildx/multi-plaform outside of core.... so then we don't need to maintain it as much.... |
| [12:44:47] | <mstenta[m]> | yea. so maybe we're trying to solve a problem that doesn't actually exist? in theory composer.lock could be different between prod and dev builds, which could hypothetically lead to something broken in prod that passes tests in dev (i think that's the problem we're talking about here ultimately, right?)... |
| [12:45:01] | <mstenta[m]> | well i do think it's worth providing official images for arm |
| [12:45:56] | <paul121[m]> | <mstenta[m]> "or.... maybe we don't need to..." <- yeah i see this. its more about providing the base images rather than a full "out of the box working env" |
| [12:46:37] | <mstenta[m]> | yea just thinking that would allow us to build a composer.lock file BEFORE we start building/testing in Docker images |
| [12:46:39] | <mstenta[m]> | so it could be shared into all of them |
| [12:46:59] | <mstenta[m]> | again... if we need to have the same composer.lock file... but maybe this isn't that big of a concern |
| [12:47:54] | <mstenta[m]> | > could hypothetically lead to something broken in prod that passes tests in dev (i think that's the problem we're talking about here ultimately, right?)... |
| [12:47:54] | <mstenta[m]> | this *would* be a very confusing issue, if it happened 😂 |
| [12:48:18] | <paul121[m]> | it seems easy enough to adapt a single composer.lock to our existing build steps... but would introduce a lot more complexity to do the same for buildx |
| [12:48:20] | <mstenta[m]> | but we would probably figure it out pretty quickly |
| [12:48:25] | <mstenta[m]> | the next push of 3.x would fail too |
| [12:48:29] | <mstenta[m]> | and then we could debug |
| [12:49:09] | <paul121[m]> | yea. like that recent consumers issue |
| [12:49:30] | <mstenta[m]> | oh did that happen in between dev/prod builds? |
| [12:49:31] | <paul121[m]> | that's basically what we're hoping to avoid |
| [12:49:39] | <paul121[m]> | no no it didn't |
| [12:49:53] | <paul121[m]> | at least I didn't notice haha |
| [12:49:59] | <mstenta[m]> | oh... well then that's a different issue |
| [12:50:11] | <mstenta[m]> | that's just because we don't commit composer.lock in the first place |
| [12:50:24] | <mstenta[m]> | all of this stuff we're talking about wouldn't avoid the consumers issue |
| [12:50:37] | <mstenta[m]> | (i don't think?) |
| [12:51:00] | <paul121[m]> | that's correct - but the consumers issue is an example of something that could happen between dev/prod builds :-) |
| [12:51:10] | <mstenta[m]> | true |
| [12:55:25] | <mstenta[m]> | whelp. without committing composer.lock anyone building farmOS with composer has the same risk... regardless of whether we catch it between dev/prod build. |
| [12:55:58] | <mstenta[m]> | i guess it's a bit out of our hands though |
| [12:56:12] | <mstenta[m]> | the line between the "app" and the "project" is blurry in that regard |
| [12:56:27] | <paul121[m]> | yeah. ideally you run the tests yourself before you push to production 😅 |
| [12:56:36] | <paul121[m]> | but that's really it |
| [12:56:46] | <paul121[m]> | or farmos pins a lot more of our dependencies |
| [12:56:56] | <mstenta[m]> | i was just typing that :-) |
| [12:56:57] | <mstenta[m]> | yup |
| [12:57:34] | <mstenta[m]> | but even that would only catch issues with top-level deps |
| [12:58:03] | <mstenta[m]> | eg: if there was some bug in a lower-level symfony package, we'd still be susceptible to that |
| [12:58:26] | <mstenta[m]> | this might be unavoidable to some degree |
| [12:58:36] | <mstenta[m]> | although it reminds me of this old PR... |
| [12:58:52] | <mstenta[m]> | https://github.com/farmOS/farmOS/pull/739 |
| [13:00:26] | <paul121[m]> | k I actually gtg now :-) |
| [13:01:04] | <mstenta[m]> | > Apart from fixing farmOS/composer-project#10, there are two other small benefits to doing this:... (full message at <https://matrix.org/_matrix/media/v3/download/matrix.org/kmmIPddsrFFlmQOt...) |
| [13:01:29] | <mstenta[m]> | Maybe that + making composer based installs the recommended path is really the best we can do |
| [13:02:18] | <mstenta[m]> | Then we would be committing composer.lock for tags at least |
| [13:18:10] | <mstenta[m]> | > Anyway, gotta run. Hopefully I'll get a chance to look into this more from my end tomorrow morning... |
| [13:18:10] | <mstenta[m]> | symbioquine FWIW everything we've been discussing is only tangentially related to your `buildx` work, and the issue we're imagining is an edge case... just to say I don't think any of this `composer.lock` discussion needs to block/affect the `buildx` PR IMO |
| [13:19:28] | <mstenta[m]> | IMO providing official ARM images provides more of a win than preventing the composer.lock edge case (and we can figure out a solution to that afterwards if we need to) |
| [13:20:12] | <mstenta[m]> | But now my gears are turning on some of the ideas in https://github.com/farmOS/farmOS/pull/739 again as a different way of solving the composer.lock issue 😄 |
| [13:21:03] | <mstenta[m]> | If we did commit composer.lock to farmOS/composer-project (only on tagged releases), then we would be certain that the build for a tag would use that lock |
| [13:21:57] | <mstenta[m]> | the issue would still be possible for 3.x images, but it's really the tagged releases we want reproducibility on first and foremost I think |
| [14:14:40] | <symbioquine[m]> | I think that's a really good compromise, personally |
| [14:17:52] | <symbioquine[m]> | <mstenta[m]> "> Anyway, gotta run. Hopefully I..." <- > <@mstenta:matrix.org> > Anyway, gotta run. Hopefully I'll get a chance to look into this more from my end tomorrow morning...... (full message at <https://matrix.org/_matrix/media/v3/download/matrix.org/NLoeXDozuysgzSlO...) |
