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 --project=@script when outside script directory #56351

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fix --project=@script when outside script directory #56351

wants to merge 6 commits into from
+30 −18

Conversation

awadell1
Copy link
Contributor

Should close #55069 and #55606

As discussed in #55069, this PR adds a program_file field to JLOptions, allowing the program file to be defined during Base.__init__() when load_path_expand is first called. The PR is broken into four commits:

  • 4c7f4dd: Add tests to replicate the above issues
  • 575cd1e: Implements the above fix
  • 2828fad: Adds --project=@script help text to the cli

@awadell1
Copy link
Contributor Author

As best I can tell, the test failure appears unrelated to this PR. Should be good for review

cc @tecosaur @Keno @oxinabox @StevenWhitaker

StevenWhitaker
StevenWhitaker reviewed Oct 28, 2024
View reviewed changes
test/cmdlineargs.jl Outdated Show resolved Hide resolved
@awadell1
add tests for --project=@script
4271c98 
Tests for JuliaLang#55606 and JuliaLang#55069
@awadell1
Add program_file to JLOptions and use for parsing project-at-script
2f63340 
As detailed in JuliaLang#55069, PROGRAM_FILE and ARGS are unset when
`load_path_expand` is first called during `Base.__init__()`, this
adds `program_file` to JLOptions as sets it during `jl_parse_opts`

Removed `unsafe_load_commands` path in `load_path_expand` as ARGS is
unset at this point.
@awadell1
Update cli docstring to mention at-script
1dab272
@awadell1
fix whitespace
be1b376
@awadell1
fix correct typo in test/cmdlineargs.jl
c142f78
@awadell1
Copy link
Contributor Author

awadell1 commented Dec 10, 2024 •
edited
Loading

Bumping as #55069 and #55606 are still present in 1.11.2

Buildkite failures appear to be unrelated and similar to what's on master

cc @tecosaur @Keno @oxinabox

tecosaur
tecosaur approved these changes Jan 18, 2025
View reviewed changes
Copy link
Contributor

@tecosaur tecosaur left a comment

Choose a reason for hiding this comment

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

Looking through the code, this looks sensible to me, though ideally somebody involved in the original PR would give this a stamp of approval and merge.

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

@tecosaur tecosaur tecosaur approved these changes

@StevenWhitaker StevenWhitaker Awaiting requested review from StevenWhitaker

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

--project=@script does not activate script project if outside of the Project Directory
4 participants
@awadell1 @StevenWhitaker @tecosaur @giordano