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

[solidity-upgrade] Input file processing is out of sync with solc #11651

Closed
cameel opened this issue Jul 13, 2021 · 4 comments
Closed

[solidity-upgrade] Input file processing is out of sync with solc #11651

cameel opened this issue Jul 13, 2021 · 4 comments
Labels
bug 🐛 easy difficulty good first issue candidate Could be a "good first issue" but something is blocking it or it has open questions.

Comments

@cameel
Copy link
Member

cameel commented Jul 13, 2021

Looks like soldity-upgrade has input file processing logic duplicated from CommandLineInterface.cpp. Especially readInputFiles() and file reader callback in SourceUpgrade.cpp. This logic in CommandLineInterface has recently been refactored in PRs like #11113, #11518. After #11545/#11617 will actually become functionally different so solidity-upgrade will be unable to resolve imports exactly the same way solc does so projects that depend on --base-path being set properly might no longer compile.

Logic needs to be unified in both places. Both should use FileReader and its callback. Preferably the input processing inside readInputFiles() itself should be shared between the two executables. I think that solidity-upgrade also needs --base-path option.
@cameel cameel mentioned this issue Jul 13, 2021
Merged
@siddharth1704
Copy link

Can you assign me on this issue i am new to open source i might need some help but i will complete the given task

@cameel
Copy link
Member Author

cameel commented Aug 19, 2021 •
edited
Loading

Sure. You might want to start by trying out the solidity-upgrade tool and seeing how it works on the command line compared to solc.

If you start a PR, it's best to do it on top of #11545 (which is in review right now) because it touches that code.

Drop by at the #solidity-dev Matrix/Gitter channel if you need more help.

@siddharth1704
Copy link

ok i will genrate the PR on top of it

@cameel cameel added good first issue candidate Could be a "good first issue" but something is blocking it or it has open questions. and removed good first issue labels Dec 11, 2021
@NunoFilipeSantos NunoFilipeSantos added the stale The issue/PR was marked as stale because it has been open for too long. label Nov 25, 2022
@cameel cameel removed the stale The issue/PR was marked as stale because it has been open for too long. label Dec 2, 2022
@cameel
Copy link
Member Author

cameel commented Dec 5, 2022

Closing due to our plans to remove solidity-upgrade from the compiler in the near future (#13715).

@cameel cameel closed this as not planned Won't fix, can't repro, duplicate, stale Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug 🐛 easy difficulty good first issue candidate Could be a "good first issue" but something is blocking it or it has open questions.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@cameel @NunoFilipeSantos @siddharth1704 and others