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

[Backport release-24.05] factorio_2 + factorio-space-age: init at 2.0.8 #350379

Merged

Conversation

bmillwood
Copy link
Contributor

@bmillwood bmillwood commented Oct 22, 2024

edit: the final version of this PR backported:

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
    • not all, just headless and the expansion binary
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@thevar1able
Copy link
Contributor

👀 #350377

@bmillwood bmillwood force-pushed the backport-350214-to-release-24.05 branch from 23e38ae to 13979f3 Compare October 22, 2024 00:41
@bmillwood
Copy link
Contributor Author

ok, creating a new PR actually didn't fix my CI checks issue, but doing git commit --amend --no-edit to change the commit hash seems to have done it

@bmillwood
Copy link
Contributor Author

👀 #350377

yeah, I just saw that. Inclined to keep this as-is and just backport that as well when it's done, for the sake of simplicity, but can do the additional cherry-picks if someone thinks that's better

@bmillwood bmillwood changed the title [Backport release-24.05] factorio: 1.1.110 -> 2.0.7 [Backport release-24.05] factorio: 1.1.107 -> 2.0.7 Oct 22, 2024
@bmillwood
Copy link
Contributor Author

bmillwood commented Oct 22, 2024

oh! the merge conflicts are because 24.05 never got 1.1.110 and was still on 1.1.107, i.e. these were not backported:

anyway, it's time for me to go to bed for the night, I'll look at this in the morning if it's not already done by someone else instead

for now I renamed the PR to reflect the real version change, but if we want to cherry-pick the full commit sequence properly I can do that tomorrow

@VuiMuich
Copy link
Contributor

👀 #350377

yeah, I just saw that. Inclined to keep this as-is and just backport that as well when it's done, for the sake of simplicity, but can do the additional cherry-picks if someone thinks that's better

As much as I appreciate the effort to backport, please allow me to raise the question if it's worth it at this point.
Wube is famous for pumping out fixes almost by the hour, after such big releases. So for the next week or two we might barely be able to keep up with that pace on master, let alone hydra delayed merges on the "public" branches.
And maintainers of factorio servers based on NixOS 24.05 will be forced to put in some manual effort anyways, to be up to date with Wube's infamous tight fixing pace after such big releases.

@bmillwood
Copy link
Contributor Author

👀 #350377

yeah, I just saw that. Inclined to keep this as-is and just backport that as well when it's done, for the sake of simplicity, but can do the additional cherry-picks if someone thinks that's better

As much as I appreciate the effort to backport, please allow me to raise the question if it's worth it at this point. Wube is famous for pumping out fixes almost by the hour, after such big releases. So for the next week or two we might barely be able to keep up with that pace on master, let alone hydra delayed merges on the "public" branches. And maintainers of factorio servers based on NixOS 24.05 will be forced to put in some manual effort anyways, to be up to date with Wube's infamous tight fixing pace after such big releases.

I'm running the game for myself personally and running a server for me and my friends, so I already have to do the necessary work here, so I might as well make PRs for it :)

@erictapen erictapen removed their request for review October 22, 2024 10:27
@bmillwood bmillwood force-pushed the backport-350214-to-release-24.05 branch from 13979f3 to 8b33d2e Compare October 22, 2024 10:28
@bmillwood bmillwood changed the title [Backport release-24.05] factorio: 1.1.107 -> 2.0.7 [Backport release-24.05] factorio: 1.1.107 -> 2.0.7, + space-age Oct 22, 2024
@bmillwood bmillwood changed the title [Backport release-24.05] factorio: 1.1.107 -> 2.0.7, + space-age [Backport release-24.05] factorio: 1.1.107 -> 2.0.7, + factorio-space-age Oct 22, 2024
@bmillwood
Copy link
Contributor Author

ok, I put in the missing backports and I think this is ready to be merged

@VuiMuich
Copy link
Contributor

👀 #350377

yeah, I just saw that. Inclined to keep this as-is and just backport that as well when it's done, for the sake of simplicity, but can do the additional cherry-picks if someone thinks that's better

As much as I appreciate the effort to backport, please allow me to raise the question if it's worth it at this point. Wube is famous for pumping out fixes almost by the hour, after such big releases. So for the next week or two we might barely be able to keep up with that pace on master, let alone hydra delayed merges on the "public" branches. And maintainers of factorio servers based on NixOS 24.05 will be forced to put in some manual effort anyways, to be up to date with Wube's infamous tight fixing pace after such big releases.

I'm running the game for myself personally and running a server for me and my friends, so I already have to do the necessary work here, so I might as well make PRs for it :)

Fair enough.

@ofborg ofborg bot requested a review from erictapen October 22, 2024 13:14
@SuperSandro2000
Copy link
Member

So we want to backport a major update which is explicitly breaking and recommend to start a new map to stable?

@erictapen erictapen removed their request for review October 22, 2024 13:28
@bmillwood
Copy link
Contributor Author

bmillwood commented Oct 22, 2024

So we want to backport a major update which is explicitly breaking and recommend to start a new map to stable?

The pop-up that recommends that you start a new game when you're loading a save is only there if you switch from base Factorio to space age, which also means switching which package you're installing, so I think it's reasonable to at least backport the new package. (I just checked and disabling the three mods that Space Age adds is sufficient to disable the warning.)

The upgrade of base factorio from 1.1.107 to 2.0.7 is certainly bigger and more fraught than the average Factorio upgrade, but I'm not sure it's of a fundamentally different kind. A bunch of recipes and entities get replaced, some technologies get changed, and there are some balance changes, but it is at least intended that people are able to upgrade their saves.

shrug My guess is that this is what people will want (it's certainly what I will want), but I can see the argument the other way. Let me know if you want any of:

  • a separate backport of base from 1.1.107 to 1.1.110
  • a backport that attempts to just add the space-age package while leaving the base package on the 1.x series

@VuiMuich
Copy link
Contributor

factorio_1 sound like a good idea.
AFAIK rails curves of 2.0 are backwards incompatible even without space age.

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Oct 22, 2024

factorio_1 sound like a good idea. AFAIK rails curves of 2.0 are backwards incompatible even without space age.

Yes and yep. That is way more inline with the backporting rules.

@lukegb
Copy link
Contributor

lukegb commented Oct 22, 2024

For the backport specifically we should have factorio_2 as a new package, and bump factorio to the latest 1.x release. This will be a bit weird when people upgrade to unstable or the next release but such is life.

@bmillwood bmillwood force-pushed the backport-350214-to-release-24.05 branch from 8b33d2e to 29747bb Compare October 23, 2024 10:45
@bmillwood
Copy link
Contributor Author

bmillwood commented Oct 23, 2024

  • This PR is now based on the new PR I opened for just the backport of the 1.x updates, [Backport release-24.05] factorio 1.1.107 -> 1.1.110 #350682
  • Unfortunately I can't set that as the base branch for this PR because the branch only exists on my fork, and the base branch for this PR has to be in this fork. So probably safe to just ignore this until the other one is merged.

@ofborg ofborg bot requested a review from erictapen October 23, 2024 12:16
@erictapen erictapen removed their request for review October 23, 2024 19:12
@bmillwood bmillwood force-pushed the backport-350214-to-release-24.05 branch from 29747bb to b53af42 Compare October 29, 2024 11:23
@bmillwood bmillwood changed the title [Backport release-24.05] factorio: 1.1.107 -> 2.0.7, + factorio-space-age [Backport release-24.05] factorio-space-age: init at 2.0.7 Oct 29, 2024
@bmillwood bmillwood force-pushed the backport-350214-to-release-24.05 branch 2 times, most recently from 7256a19 to 08bd213 Compare October 29, 2024 12:12
@bmillwood bmillwood marked this pull request as draft October 29, 2024 12:13
@bmillwood
Copy link
Contributor Author

bmillwood commented Oct 29, 2024

ok, the latest here is:

  • _1 and _2-suffixed versions of everything added (except for the demo, which doesn't have a 2.x release)
  • this is by moving versions.json to versions-1.json and adding versions.json for the v2 versions (the naming is such that future cherry-picks update the correct file)
  • the _1 packages are defined in terms of the unversioned packages instead of vice versa just so that the by-name CI check will leave me alone
  • I haven't backported any of the upgrades beyond 2.0.8 because after that master moves factorio to the by-name hierarchy, which would make the diff here more complex / less readable I think; happy to do a followup for that

I'm mindful that nixos-24.11 is only about a month away, so if this is a tonne of effort, maybe it'll end up being not worth it, but honestly I'm interested in getting practice doing this kind of thing, so for my own efforts I don't really mind

@bmillwood bmillwood changed the title [Backport release-24.05] factorio-space-age: init at 2.0.7 [Backport release-24.05] factorio_2 + factorio-space-age: init at 2.0.8 Oct 29, 2024
bmillwood and others added 5 commits October 29, 2024 14:55
Freeze versions into a separate versions.json for _1; there are unlikely
to be any more releases in that series, and this means that
cherry-picking version updates from master will update the correct file.

So far the _2 packages are misnamed because I haven't done the
cherry-picking yet, but that will be fixed next.
(cherry picked from commit 56444f5)
(cherry picked from commit 5579fe3)
space-age needs to be based on `factorio_2`, not `factorio`
Note that this also deletes factorio-space-age-experimental; this is
left in top-level/all-packages.nix for now since I don't know if they
plan to bring it back. Longer term we should make it resolve to stable
if it doesn't exist, probably.

(cherry picked from commit ed88329)
@bmillwood bmillwood force-pushed the backport-350214-to-release-24.05 branch from 883f4b0 to ec4ab2e Compare October 29, 2024 14:58
@ofborg ofborg bot requested a review from erictapen October 29, 2024 19:12
@bmillwood bmillwood marked this pull request as ready for review October 30, 2024 01:27
@lukegb lukegb merged commit 918bd8d into NixOS:release-24.05 Oct 30, 2024
28 of 29 checks passed
@bmillwood
Copy link
Contributor Author

thanks Luke, appreciate it ❤️

@erictapen erictapen removed their request for review October 30, 2024 11:09
@bmillwood bmillwood deleted the backport-350214-to-release-24.05 branch October 30, 2024 13:28
@bmillwood
Copy link
Contributor Author

I got two GitHub Action failure notifications for this PR:

https://github.com/NixOS/nixpkgs/actions/runs/11594177669
https://github.com/NixOS/nixpkgs/actions/runs/11594177693

Both of them sound like GitHub was still trying to run pull request actions after the PR was closed and I deleted my branch, and that was leading to it failing to check out the changes? I plan to ignore them unless someone thinks otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants