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

Add support for new Composer builder #283

Merged
merged 1 commit into from
Sep 29, 2024

Conversation

patka-123
Copy link
Contributor

A new version of buildComposerProject has been introduced. Updating packages that use this new version with nix-update failed to update the vendorHash before this PR.

@Mic92
Copy link
Owner

Mic92 commented Sep 22, 2024

Just run nix fmt locally and amend your commit. Than it should be fine.

@patka-123 patka-123 force-pushed the build-composer-project-v2 branch from 5de717a to 14c8b68 Compare September 22, 2024 16:51
@Mic92
Copy link
Owner

Mic92 commented Sep 22, 2024

now only the composer test fails.

@patka-123
Copy link
Contributor Author

Hmm, I tested both versions manually to verify it works. I can't seem to figure out what's wrong with the test though. It's probably something silly.

Do you have an idea?

patka-123 added a commit to patka-123/nix-update that referenced this pull request Sep 28, 2024
The current nixpkgs is quite old and doesn't have php.buildComposerProject2, which is needed for PR Mic92#283
@patka-123 patka-123 mentioned this pull request Sep 28, 2024
@patka-123
Copy link
Contributor Author

It was indeed silly ^^. The nixpkgs was too old and didn't have php.buildComposerProject2 in it. That's updated in #285 and as soon as that's merged the broken test in this PR will also be fixed.

@Mic92
Copy link
Owner

Mic92 commented Sep 28, 2024

@mergify rebase

Copy link
Contributor

mergify bot commented Sep 28, 2024

rebase

✅ Branch has been successfully rebased

@Mic92 Mic92 force-pushed the build-composer-project-v2 branch from 14c8b68 to 2278b7d Compare September 28, 2024 12:36
@Mic92
Copy link
Owner

Mic92 commented Sep 28, 2024

@mergify queue

Copy link
Contributor

mergify bot commented Sep 28, 2024

queue

🛑 The pull request has been merged manually

The pull request has been merged manually at d033f05

@patka-123
Copy link
Contributor Author

Did something break, or is this normal mergify behaviour? I have no idea, just curious.

@Mic92 Mic92 merged commit d033f05 into Mic92:master Sep 29, 2024
5 checks passed
@Mic92
Copy link
Owner

Mic92 commented Sep 29, 2024

I don't know the conditions are met as far as I can see.

@patka-123 patka-123 deleted the build-composer-project-v2 branch September 29, 2024 09:04
@RafaelKr
Copy link

RafaelKr commented Nov 9, 2024

@patka-123 Maybe using composer1 instead of composer-old would be more appropriate as there will be coming more versions in the future. But maybe the time to change that is when composer 3 is released.

@patka-123
Copy link
Contributor Author

I followed the go example that is already present in this repo.

And I'm not sure what you mean specifically. This isn't about composer itself, but the buildComposerProject within nixpkgs. We now have two versions (where one will probably be deprecated and perhaps removed at some point).

Even in the worst case we somehow end up with 3 seperate versions of buildComposerProject we can just rename it within this project. The "old" naming is internal to this repo.

@RafaelKr
Copy link

RafaelKr commented Nov 9, 2024

Gotcha, I thought it's composer1/composer2 related, but if it's only the derivation builder of course my comment doesn't apply. Thanks for clarifying.

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.

3 participants