Skip to content

Conversation

@crazywhalecc
Copy link
Owner

What does this PR do?

Add skeleton command.

Checklist before merging

If your PR involves the changes mentioned below and completed the action, please tick the corresponding option.
If a modification is not involved, please skip it directly.

  • If you modified *.php or *.json, run them locally to ensure your changes are valid:
    • composer cs-fix
    • composer analyse
    • composer test
    • bin/spc dev:sort-config
  • If it's an extension or dependency update, please ensure the following:
    • Add your test combination to src/globals/test-extensions.php.
    • If adding new or fixing bugs, add commit message containing extension test or test extensions to trigger full test suite.

@crazywhalecc crazywhalecc added the mixed PR This PR contains multiple updates label Dec 15, 2025
@crazywhalecc
Copy link
Owner Author

image

🎉

@crazywhalecc crazywhalecc marked this pull request as ready for review December 18, 2025 07:46
@henderkes
Copy link
Collaborator

I need to push something to fix my space problem in CI quick and then review this.

@crazywhalecc
Copy link
Owner Author

crazywhalecc commented Dec 18, 2025

I need to push something to fix my space problem in CI quick and then review this.

It's still WIP(target and extension implementations is not available) yet. But the structure of generator is done. I'll do remaining part in separate PRs. My local has a lot of uncommited changes though.

But I'm open to it if you willing to review this now.

@henderkes
Copy link
Collaborator

There's a general thing I'm not entirely clear on yet and it's the distinction between library, extension and "package" which seems could be either? I'm not sure if unifying them into packages so prominently is such a good idea.

We should also rename #BuildFor to #When[os: ['Linux', 'Darwin'] (or two #Whens.

Mimicking Symfony a bit.

@crazywhalecc
Copy link
Owner Author

There's a general thing I'm not entirely clear on yet and it's the distinction between library, extension and "package" which seems could be either? I'm not sure if unifying them into packages so prominently is such a good idea.

Unifying them benefits the dependency parsing. From v2 we did the same thing on dependency resolver.

php-extension is a virtual package finally. Maintaining 2 lists (artifact, package) is always better than 5 (source, pre-built, pkg, lib, ext)

@henderkes
Copy link
Collaborator

That's why I meant "so prominently". In my opinion that should be hidden behind a common "package" trait, not by declaring everything a package directly. I'd rather have #Library (which implements #Package) and #Extension (which also implements #Package) because they do have fundamental difference in the way they are built, even if not in terms of binary production/distribution.

@crazywhalecc
Copy link
Owner Author

I'd rather have #Library (which implements #Package) and #Extension (which also implements #Package)

Sorry I don't get it. Declaring them as a package is no ambigious I think. What's your perspective or target?

@henderkes
Copy link
Collaborator

I'd rather have #Library (which implements #Package) and #Extension (which also implements #Package)

Sorry I don't get it. Declaring them as a package is no ambigious I think. What's your perspective or target?

Essentially that libraries and extensions aren't the same. They don't build the same way or at the same time, so we shouldn't declare them the same way. I think the split between ext.json and lib.json makes a lot of sense (source.json is debatable, probably not).

@crazywhalecc
Copy link
Owner Author

I think the split between ext.json and lib.json makes a lot of sense

Config level we already separated them after all.

Essentially that libraries and extensions aren't the same

True. So these package types are different. But they are resolving dependency on the same level.

They don't build the same way or at the same time

Of course they don't. The library and target type of package is handled by PackageInstaller and php extension package(non-library or parent type) will be skipped in it. Extensions will be resolved in php package.

That is, if someone wants to reuse spc dependencies in vendor mode to build a thing rather than php, it will be easier to extend the package type ,like "xxx-extension". Anyway it's clearer to declare dependency relations.

@crazywhalecc
Copy link
Owner Author

We should also rename #BuildFor to #When[os: ['Linux', 'Darwin'] (or two #Whens.

If we have When, then it should be equivalent to #[Stage('build')] + #[When(os: '['Linux]')]. #BuildFor() is defined primarily for simplicity, essentially as an alias. But you have a point: we might eventually need a separate #[When] attribute.

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

Labels

mixed PR This PR contains multiple updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants