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

Make scalac's argument-file processing more like javac's in handling spaces and line breaks #10319

Merged
merged 1 commit into from Mar 28, 2023

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Feb 19, 2023

Fixes scala/bug#12732; the ticket has details on the behavior differences

@scala-jenkins scala-jenkins added this to the 2.13.11 milestone Feb 19, 2023
@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Feb 19, 2023
@michelou
Copy link

michelou commented Feb 19, 2023

@som-snytt Your changes work for me, thanks. I closed PR #10316 in favor of your PR after testing your changes with several Scala 2 code examples. I also patched the 2.13.10 installation (i.e. scala-compiler.jar) on my machines until version 2.13.11 is available.

@som-snytt
Copy link
Contributor Author

@michelou Thanks for trying out the fix. I will probably not further tweak to be bug-compatible with Java.

To summarize, the change in 2.13.10 arrived in 2.13.9 but deemed too late in the cycle to refine, so it was reverted and deferred, but then did not receive further scrutiny; I see it got the "release notes" label, but only the -Vprint-args feature was highlighted.

I was hoping to use the same tokenizer for sys.process and arg files. That is a pipe dream. (sys.process pun.)

I'd forgotten this bug mentioned in my PR, which makes it sound like JDK has quoting bugs in both places:
https://bugs.openjdk.org/browse/JDK-8282008

The context is complicated by the reverts, but the original PRs were:
#10114
#10123

@SethTisue SethTisue added the prio:hi high priority (used only by core team, only near release time) label Feb 20, 2023
@som-snytt som-snytt marked this pull request as ready for review February 22, 2023 19:17
Copy link
Member

@SethTisue SethTisue left a comment

Choose a reason for hiding this comment

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

thanks @michelou for bringing this to our attention, and thanks @som-snytt for untangling the history

@SethTisue SethTisue changed the title Re-revert untokenized argfile, write quoted args Make scalac's argument-file processing more like javac's Mar 13, 2023
@SethTisue
Copy link
Member

SethTisue commented Mar 13, 2023

@som-snytt I improved the title and PR description a bit. not sure whether to put additional effort into it; on the one hand it's an incompatible change, so people need to know, but otoh I doubt the feature is widely used

@SethTisue SethTisue removed the prio:hi high priority (used only by core team, only near release time) label Mar 13, 2023
@SethTisue SethTisue changed the title Make scalac's argument-file processing more like javac's Make scalac's argument-file processing more like javac's in handling spaces and line breaks Mar 13, 2023
@SethTisue SethTisue removed their assignment Mar 13, 2023
@som-snytt
Copy link
Contributor Author

@SethTisue yes, I think is really a reversion. If I could go back in time and live my life over again, this is what I would do differently.

I only work on features that nobody uses, so it should be safe.

@SethTisue SethTisue self-assigned this Mar 13, 2023
@michelou
Copy link

@SethTisue Just one reminder to make sure this fix will go into release 2.13.11 : Kotlin and Scala 3 do behave the same as Java in respect to the handling of argument files passed on the command line.

PS. I make use of argument files in most build scripts (bash, batch, Makefile) available from my Github repositories (e.g. kotlin-examples, dotty-examples, akka-examples, etc..) when invoking the commands javac, kotlinc, and scalac (v3).

@SethTisue SethTisue added the prio:hi high priority (used only by core team, only near release time) label Mar 22, 2023
@SethTisue

This comment was marked as resolved.

@SethTisue SethTisue merged commit a256d69 into scala:2.13.x Mar 28, 2023
1 check passed
@SethTisue SethTisue removed the prio:hi high priority (used only by core team, only near release time) label Mar 28, 2023
@som-snytt som-snytt deleted the tweak/argfiles branch March 28, 2023 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
5 participants