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 missing dependencies #888

Merged
merged 1 commit into from Aug 14, 2023
Merged

Add missing dependencies #888

merged 1 commit into from Aug 14, 2023

Conversation

neonichu
Copy link
Member

No description provided.

@neonichu
Copy link
Member Author

@swift-ci please test

@neonichu
Copy link
Member Author

@swift-ci please smoke test

@neonichu neonichu merged commit 02c9df6 into apple:main Aug 14, 2023
5 checks passed
@neonichu neonichu deleted the fix-deps branch August 14, 2023 20:59
@finagolfin
Copy link
Contributor

@neonichu, why is this llbuild executable being built as a dependency here and in apple/swift-package-manager#6809 now? It just broke my Android CI when cross-compiling trunk for armv7, which I can fix, but I wonder if you intend to build this executable or need the underlying libraries instead.

@neonichu
Copy link
Member Author

Sorry about that, yes I meant libllbuild. I was using --explicit-target-dependency-import-check, it's possible that it gave me incorrect information which I did not verify sufficiently.

@neonichu
Copy link
Member Author

Yah, indeed I am seeing

[error]: Target llbuildAnalysis imports another target (llbuild) in the package without declaring it a dependency.
[error]: Target llbuildSwift imports another target (llbuild) in the package without declaring it a dependency.

@neonichu
Copy link
Member Author

neonichu commented Aug 25, 2023

We'll likely have to revert apple/swift-package-manager#6810 if we fix this since it'll run into that. That's not a problem per se, just mentioning it. I was basically trying to use this in order to determine if it is ready for more widespread usage and it seems like I got my answer for now.

@finagolfin
Copy link
Contributor

I was able to work around this for trunk on my Android CI, finagolfin/swift-android-sdk#119.

@finagolfin
Copy link
Contributor

@neonichu, should I submit pulls to revert this or do you plan to do it? Just let me know.

@neonichu
Copy link
Member Author

@finagolfin sorry, I forgot about this - would be great if you could do it, thanks!

finagolfin added a commit to finagolfin/swift-llbuild that referenced this pull request Sep 16, 2023
The llbuild executable is not a dependency of these two libraries.
owenv added a commit that referenced this pull request Sep 26, 2023
Revert 'Add missing dependencies (#888)'
finagolfin added a commit to finagolfin/swift-llbuild that referenced this pull request Oct 3, 2023
The llbuild executable is not a dependency of these two libraries.
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.

None yet

3 participants