-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[breaking change] Stop supporting script tag based execution of part files #52575
Comments
fine by me |
For what it's worth, I still think that shebangs on part files should not be a syntax error. I think that they should be "ignored" in a similar way how comments are meant to be ignored by Dart compilers, and only act as convenient syntax for embedding shell/terminal-related information into Dart programs that is ignored by Dart-related tooling. This would allow for a simpler mental model of Dart syntax, where:
Conditionally supporting the "shell-related language" only if the innermost "actual dart program" does not have a part header seems unnecessarily restrictive and I really don't understand why this is considered to be a bug. (Or is the ability to execute part files the actual bug?) In other words, I think that a script tag at the top of a part file should not cause its library to fail compilation (if the library was executed properly) and I agree that there's no good reason to support executing part files. |
@modulovalue wrote:
That's very simple: The specification contains a grammar, that grammar doesn't derive the part file contents whose first line is a script tag, and hence a would-be part file that starts with a script tag is not a part file, it's a syntax error. In contrast, a This could be a bug in the parser, or it could be a bug in the specification (if such script tags are useful, and they were omitted from the grammar rule by mistake). During discussions about this topic I haven't heard any arguments in favor of adding support for these script tags to the specification, until now — your comment is exactly such an argument:
That is indeed a nice, complete perspective on Dart source code artifacts. I think you could say that this model of Dart source code artifacts is similar to a model that we frequently use with packets on a network (linking to figure on https://www.cs.ait.ac.th/~on/O/oreilly/tcpip/firewall/ch06_03.htm): However, I'm still not convinced that this apparently general and all-covering perspective brings anything particularly useful to Dart. First, as you mentioned, the leading 0xFEFF (byte-order mark) is specifically part of the encoding. You could argue that it should be a built-in property of the lexical analyzer that it reads this byte-order mark and processes it as "not part of the encoded document, only part of the encoding". It's there because of the history, but we might want to fix it (such that the grammar doesn't mention the byte-order mark any more). Next, the script tag: This is indeed very similar to the network packet structure, where the script-tag-processor ignores the rest of the file, passing it on as "data" to the next tool. However, I don't see much value in allowing script tags in part files, especially if they won't ever work (as soon as we fix this issue, of course ;-). The magic language version comments (like We could use that kind of layered model of the Dart source code artifacts, but it is not obvious to me that the effort that we'd put into an implementation of this model would bring any clear benefits.
Alas, but why not? 😀 |
Thank you for the explanation and I agree with your view that this is a bug. I was too imprecise in my wording and meant to rather imply that I disagree with making the presence of a script tag in a part file a compile-time error.
I left a similar comment here in the last section of the issue description. (It might not have been as clear as the one that I left here.) On the script tag:
Have you considered that the script tag can contain strings that are unrelated to dart tooling? Consider this example:
outputs
this is an example of something that "would work" even if part files themselves could not be ran the way that they can be today. We are not limited to providing the script tag with the path to a dart binary, but we can specify any shell script. On the language version string:
By doing that we would fix a minor inconsistency that I've pointed out in the following issue: dart-lang/language#2945. (Consider the following for exposition purposes:)
I think that by not allowing
|
I don't anticipate this having any impact to ACX. |
@modulovalue, you're fighting hard for the ability to have a script tag in a part file. ;-) Or, as a different perspective, to structure the language in layers, somewhat similar to network packets, and use the same over structure for libraries and part files. However, I don't think the idea has much support. So I'd recommend that we go ahead with this breaking change and make it a syntax error (as specified) for a part file to have a script tag. The analyzer already reports a compile-time error when it encounters a script tag in a part file, but the CFE would need to be modified such that the error is reported (and the code that actually allows it to run can be deleted). @dart-lang/language-team, @johnniwinther, @itsjustkevin, do you wish to have further steps in the breaking change process, or can we simply go ahead and do it? I believe this can be a CFE-only modification, and I suspect that the actual breakage is tiny. |
I agree that it doesn't seem like a high impact decision, but this exception looks to me like a cut in the figurative Death by a Thousand Cuts. And the fact that network packets employ a similar design seems like strong evidence to me that that would be way to go. |
I can't imagine any positive user value coming from allowing shebang lines in part files which aren't runnable Dart scripts anyway. If you want a file that:
Might I suggest giving it the file extension |
For the rest of eternity the analyzer (and compiler) will have to check whether a file is a part file with a script tag only to emit an error (that likely nobody will ever see, I agree!), and everybody will have to pay a price for breaking a 30+ year old convention. Or they could just ignore script tags if they are the first token (or the second one if the first token is FEFF), like they already do with non-part files and keep things simpler and backwards compatible for everybody. There are people who seem to rely on hashbangs for detecting filetypes example a example b example c. There are over two million dart developers. Why risk causing churn and making people unhappy while breaking a well-supported convention? Again, to be clear, I'm not talking about executing part files, but about removing the syntax for hashbangs/shebangs/script tags in part files. Am I guilty of bikeshedding here? Maybe, but I don't think that the arguments that I've provided so far are wrong. |
lgtm |
I don't believe it's a 30 year old convention that every file format must allow an initial line starting with An example of adding support for Anyway, even if we allow |
@johnniwinther, this breaking change has been approved. Can you find a slot in the pipeline to do it? |
In response to #52575 Change-Id: I452575517c378dda3000b8fcbf4f66274b1583a6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/315420 Reviewed-by: Chloe Stefantsova <[email protected]> Commit-Queue: Johnni Winther <[email protected]>
Thanks! |
Intended change in behavior
Currently,
dart
allows a part file to be treated as a script (where a 'script' is spec-ese meaning "a library that determines an executable program", aka "an entry point"), and it allows a script tag to occur as the first line of the part file. For example:Here is the part file 'n016part.dart':
Here is the library 'n016.dart':
This issue is a request for a breaking bug fix: The CFE should report part files like 'n016part.dart' as a syntax error (or as any other compile-time error, if that is more convenient), because the grammar rules deriving the contents of a part file does not include script tags.
The analyzer already reports a
directive_after_declaration
error on 'n016part.dart', which could be taken as evidence that the breakage is minimal. However, the analyzer does not report any errors on 'n016.dart', in spite of the fact that 'n016.dart' cannot be analyzed without loading 'n016part.dart'.Justification for making this change
First, it seems very likely that the change is only minimally breaking, because of the error reported by the analyzer.
Next, the ability to run a part file as a script might be useful in the case where it could be used to run many different variants of the associated entry point. However, this mechanism does not work unless the part file uses a URI in its part-of directive, which means that the part file uniquely identifies the entry point, and the entry point again uniquely identifies the part file. In other words, we can not use this feature to support convenient execution of many different variants of the given program. All in all, this (accidental) feature does not seem to bring any value.
The migration advice would be: "Put the script tag in the entry point rather than in the part file, and then execute the entry point."
Expected impact of this change
The expected impact is minimal, given that the analyzer already reports an error when the part file with a script tag is analyzed.
The migration advice illustrates that it is rather easy to adapt current scripts using this accidental feature. Also, it is not easy to find part files in existing source code where this accidental feature is used today. There are probably very few of them.
Mitigating the change
For an existing part file with a script tag, move the script tag to the corresponding entry point. Executions of the part file as an OS level executable program file should then be replaced by execution of the corresponding entry point. Permissions may need to be adjusted such that the part file can no longer be executed (optionally), and the entry point can be executed (necessarily, otherwise the script tag will be ignored).
If the changes to the execution commands are inconvenient, it may be useful to rename the entry point and part file such that the execution commands do not need to be updated.
The text was updated successfully, but these errors were encountered: