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.

Sign up for GitHub

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

Jump to bottom

Fix solana-address-lookup-table-program compilation #34353

Merged
merged 3 commits into from
Dec 7, 2023
Merged

Fix solana-address-lookup-table-program compilation #34353

merged 3 commits into from
Dec 7, 2023

Conversation

acheroncrypto
Copy link
Contributor

Problem

solana-address-lookup-table-program crate causes compile errors in programs after #33165

Problem explained in more detail in #34352

Summary of Changes

  • Add #[cfg(target_os = "solana")] checks to decide which SDK crate to use
  • Make processor module only available in non-program environments

Fixes #34352

@mergify mergify bot added community Community contribution need:merge-assist labels Dec 7, 2023
@mergify mergify bot requested a review from a team December 7, 2023 12:24
acheroncrypto
acheroncrypto commented Dec 7, 2023
View reviewed changes
programs/address-lookup-table/src/lib.rs Outdated
Comment on lines 20 to 23
#[deprecated(
since = "1.17.0",
note = "Please use `solana_program::address_lookup_table` instead"
)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The #[deprecated] attribute doesn't seem to be doing anything for use statements as I don't get any warnings about depreciation for these re-exports. Still, it could be useful to see it when looking at the source code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is an annoyance with Rust... I really wish these did something, but I agree these are useful

programs/address-lookup-table/src/lib.rs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could probably just export from solana-program to avoid extra cfg checks since solana-sdk just re-exports from solana-program but seperation seems more organized.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, this is the way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 2bce424

joncinque
joncinque reviewed Dec 7, 2023
View reviewed changes
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noticing and fixing! I agree that it's best to just re-export from solana_program -- would you mind making that change too?

Also, I imagine this needs to be backported to 1.17, correct?

programs/address-lookup-table/src/lib.rs Outdated
Comment on lines 20 to 23
#[deprecated(
since = "1.17.0",
note = "Please use `solana_program::address_lookup_table` instead"
)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is an annoyance with Rust... I really wish these did something, but I agree these are useful

programs/address-lookup-table/src/lib.rs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, this is the way.

@acheroncrypto
Copy link
Contributor Author

Thanks for noticing and fixing!

Thanks for the review!

Also, I imagine this needs to be backported to 1.17, correct?

Yes, all releases since 1.17.0 have this problem.

@joncinque joncinque added the CI Pull Request is ready to enter CI label Dec 7, 2023
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Dec 7, 2023
@joncinque joncinque added the v1.17 PRs that should be backported to v1.17 label Dec 7, 2023
@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 7, 2023

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

joncinque
joncinque approved these changes Dec 7, 2023
View reviewed changes
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking care of this, and sorry for the breakage!

@joncinque joncinque merged commit 46921b3 into solana-labs:master Dec 7, 2023
37 checks passed
mergify bot pushed a commit that referenced this pull request Dec 7, 2023
@acheroncrypto @mergify
Fix solana-address-lookup-table-program compilation (#34353)
4eb46ad 
* Add  checks to decide which SDK crate to use

* Make  module only available in non-program environments

* Remove `solana-sdk` export and only export from `solana-program`

(cherry picked from commit 46921b3)
@mergify mergify bot mentioned this pull request Dec 7, 2023
Merged
joncinque pushed a commit that referenced this pull request Dec 7, 2023
@mergify @acheroncrypto
v1.17: Fix solana-address-lookup-table-program compilation (backpor…
d1858aa 
…t of #34353) (#34357)

Fix `solana-address-lookup-table-program` compilation (#34353)

* Add  checks to decide which SDK crate to use

* Make  module only available in non-program environments

* Remove `solana-sdk` export and only export from `solana-program`

(cherry picked from commit 46921b3)

Co-authored-by: acheron <98934430+acheroncrypto@users.noreply.github.com>
@HaoranYi HaoranYi mentioned this pull request Apr 8, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joncinque joncinque joncinque approved these changes

Assignees
No one assigned
Labels
community Community contribution need:merge-assist v1.17 PRs that should be backported to v1.17
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

solana-address-lookup-table-program compile error in program environment
3 participants
@acheroncrypto @joncinque @solana-grimes