libexpr: Deprecate CR and CRLF line endings

They are broken beyond repair, and barring a language version revision
this is our only option besides doing nothing about it.

Change-Id: I25fa4f032ca9b5ca67356946bcd889289583592e

doc/manual/rl-next/deprecated-features.md

@@ -0,0 +1,12 @@
+---
+synopsis: 'more deprecated features'
+issues: []
+cls: []
+category: Breaking Changes
+credits: [piegames, horrors]
+---
+
+This release cycle features a new batch of deprecated (anti-)features.
+You can opt in into the old behavior with `--extra-deprecated-features` or any equivalent configuration option.
+
+- `cr-line-endings`: Current handling of CR (`\r`) or CRLF (`\r\n`) line endings in Nix is inconsistent and broken, and will lead to unexpected evaluation results with certain strings. Given that fixing the semantics might silently alter the evaluation result of derivations, the only option at the moment is to disallow them alltogether. More proper support for CRLF is planned to be added back again in the future. Until then, all files must use `\n` exclusively.

lix/libexpr/parser/grammar.hh

@@ -8,7 +8,7 @@
 #include <boost/container/small_vector.hpp>
 
 // NOTE
-// nix line endings are \n, \r\n, \r. the grammar does not use eol or
+// nix line endings are \n, \r\n (deprecated), \r (deprecated). the grammar does not use eol or
 // eolf rules in favor of reproducing the old flex lexer as faithfully as
 // possible, and deferring calculation of positions to downstream users.
 
@@ -179,8 +179,20 @@ struct uri : seq<
     plus<c::uri_rest>
 > {};
 
+struct _eol {
+    struct deprecated_cr_crlf : seq<one<'\r'>, opt<one<'\n'>>> {};
+};
+
+// LF, CRLF, CR. All but \n throw a deprecation error by default.
+struct eol : _eol, sor<
+    one<'\n'>,
+    _eol::deprecated_cr_crlf
+> {};
+
+// Spacing including comments
 struct sep : sor<
-    plus<one<' ', '\t', '\r', '\n'>>,
+    plus<one<' ', '\t'>>,
+    eol,
     seq<one<'#'>, star<not_one<'\r', '\n'>>>,
     seq<string<'/', '*'>, until<string<'*', '/'>>>
 > {};
@@ -202,7 +214,7 @@ struct expr;
 struct _string {
     template<typename... Inner>
     struct literal : semantic, seq<Inner...> {};
-    struct cr_lf : semantic, seq<one<'\r'>, opt<one<'\n'>>> {};
+    struct cr_crlf : semantic, seq<one<'\r'>, opt<one<'\n'>>> {};
     struct interpolation : semantic, seq<
         p::string<'$', '{'>, seps,
         must<expr>, seps,
@@ -215,7 +227,7 @@ struct string : _string, seq<
     star<
         sor<
             _string::literal<plus<not_one<'$', '"', '\\', '\r'>>>,
-            _string::cr_lf,
+            _string::cr_crlf,
             _string::interpolation,
             _string::literal<one<'$'>, opt<one<'$'>>>,
             seq<one<'\\'>, _string::escape>
@@ -236,6 +248,7 @@ struct _ind_string {
     struct escape : semantic, must<any> {};
     /* Marker for non-empty lines */
     struct has_content : semantic, seq<> {};
+    struct cr : semantic, one<'\r'> {};
 };
 struct ind_string : _ind_string, seq<
     TAO_PEGTL_STRING("''"),
@@ -253,12 +266,13 @@ struct ind_string : _ind_string, seq<
                         _ind_string::literal<
                             plus<
                                 sor<
-                                    not_one<'$', '\'', '\n'>,
+                                    not_one<'$', '\'', '\n', '\r'>,
                                     // TODO probably factor this out like the others for performance
-                                    seq<one<'$'>, not_one<'{', '\'', '\n'>>,
+                                    seq<one<'$'>, not_one<'{', '\'', '\n', '\r'>>,
                                     seq<one<'$'>, at<one<'\n'>>>,
-                                    seq<one<'\''>, not_one<'\'', '$', '\n'>>,
-                                    seq<one<'\''>, at<one<'\n'>>>
+                                    seq<one<'\''>, not_one<'\'', '$', '\n', '\r'>>,
+                                    seq<one<'\''>, at<one<'\n'>>>,
+                                    _ind_string::cr
                                 >
                             >
                         >,

lix/libexpr/parser/parser-impl1.inc.cc

@@ -215,6 +215,13 @@ public:
 template<typename Rule>
 struct BuildAST : grammar::v1::nothing<Rule> {};
 
+template<> struct BuildAST<grammar::v1::t::eol::deprecated_cr_crlf> {
+    static void apply(const auto & in, auto &, State & ps) {
+        if (!ps.featureSettings.isEnabled(Dep::CRLineEndings))
+            ps.badLineEndingFound(ps.at(in), true);
+    }
+};
+
 struct SimpleLambdaState : SubexprState {
     using SubexprState::SubexprState;
 
@@ -552,9 +559,12 @@ template<typename... Content> struct BuildAST<grammar::v1::string::literal<Conte
     }
 };
 
-template<> struct BuildAST<grammar::v1::string::cr_lf> {
+template<> struct BuildAST<grammar::v1::string::cr_crlf> {
     static void apply(const auto & in, StringState & s, State & ps) {
-        s.append(ps.at(in), in.string_view()); // FIXME compat with old parser
+        if (!ps.featureSettings.isEnabled(Dep::CRLineEndings))
+            ps.badLineEndingFound(ps.at(in), false);
+        else
+            s.append(ps.at(in), in.string_view()); // FIXME compat with old parser
     }
 };
 
@@ -620,6 +630,13 @@ template<> struct BuildAST<grammar::v1::ind_string::has_content> {
     }
 };
 
+template<> struct BuildAST<grammar::v1::ind_string::cr> {
+    static void apply(const auto & in, IndStringState & s, State & ps) {
+        if (!ps.featureSettings.isEnabled(Dep::CRLineEndings))
+            ps.badLineEndingFound(ps.at(in), false);
+    }
+};
+
 template<> struct BuildAST<grammar::v1::ind_string> : change_head<IndStringState> {
     static void success(const auto & in, IndStringState & s, ExprState & e, State & ps) {
         e.pushExpr(noPos, ps.stripIndentation(ps.at(in), std::move(s.lines)));

lix/libexpr/parser/state.hh

@@ -31,10 +31,12 @@ struct State
     PosTable::Origin origin;
     const Expr::AstSymbols & s;
     const FeatureSettings & featureSettings;
+    bool hasWarnedAboutBadLineEndings = false; // State to only warn on first occurrence
 
     void dupAttr(const AttrPath & attrPath, const PosIdx pos, const PosIdx prevPos);
     void dupAttr(Symbol attr, const PosIdx pos, const PosIdx prevPos);
     void overridesFound(const PosIdx pos);
+    void badLineEndingFound(const PosIdx pos, bool warnOnly);
     void addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ptr<Expr> e, const PosIdx pos);
     void validateLambdaAttrs(AttrsPattern & pattern, PosIdx pos = noPos);
     std::unique_ptr<Expr> stripIndentation(const PosIdx pos, std::vector<IndStringLine> && line);
@@ -90,6 +92,29 @@ inline void State::overridesFound(const PosIdx pos) {
     );
 }
 
+// Added 2025-02-05. This is unlikely to ever occur in the wild, given how broken it is
+inline void State::badLineEndingFound(const PosIdx pos, bool warnOnly)
+{
+    // Within strings we should throw because it is a correctness issue, outside of
+    // strings it only harmlessly fucks up line numbers in error messages so warning is sufficient.
+    if (warnOnly) {
+        if (!hasWarnedAboutBadLineEndings)
+            warn(
+                "CR (`\\r`) and CRLF (`\\r\\n`) line endings found at %s. Please inspect the file and normalize it to use LF (`\\n`) line endings instead. Use %s to silence this warning.",
+                positions[pos],
+                "--extra-deprecated-features cr-line-endings"
+            );
+        hasWarnedAboutBadLineEndings = true;
+    } else
+        throw ParseError({
+            .msg = HintFmt(
+                "CR (`\\r`) and CRLF (`\\r\\n`) line endings are not supported. Please inspect the file and normalize it to use LF (`\\n`) line endings instead. Use %s to silence this warning.",
+                "--extra-deprecated-features cr-line-endings"
+            ),
+            .pos = positions[pos],
+        });
+}
+
 inline void State::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ptr<Expr> e, const PosIdx pos)
 {
     AttrPath::iterator i;

lix/libutil/deprecated-features/cr-line-endings.md

@@ -0,0 +1,6 @@
+---
+name: cr-line-endings
+internalName: CRLineEndings
+---
+Allow CR (`\r`) and CRLF (`\r\n`) as line delimiters.
+Note however that the implementation is inconsistent and buggy and may lead to unexpected evaluation results with certain strings.

lix/libutil/meson.build

@@ -155,6 +155,7 @@ experimental_feature_definitions = files(
 
 deprecated_feature_definitions = files(
   'deprecated-features/ancient-let.md',
+  'deprecated-features/cr-line-endings.md',
   'deprecated-features/rec-set-overrides.md',
   'deprecated-features/url-literals.md',
   'deprecated-features/shadow-internal-symbols.md',

tests/functional/lang/eval-okay-unsafeGetAttrPos.flags

@@ -0,0 +1 @@
+--extra-deprecated-features cr-line-endings

tests/functional/lang/parse-fail-crlf.err.exp

@@ -0,0 +1,7 @@
+warning: CR (`\r`) and CRLF (`\r\n`) line endings found at «stdin»:7:21. Please inspect the file and normalize it to use LF (`\n`) line endings instead. Use --extra-deprecated-features cr-line-endings to silence this warning.
+error: CR (`\r`) and CRLF (`\r\n`) line endings are not supported. Please inspect the file and normalize it to use LF (`\n`) line endings instead. Use --extra-deprecated-features cr-line-endings to silence this warning.
+       at «stdin»:14:15:
+           13|   # translated to LF.
+           14|   foo = "multi
+             |               ^
+           15| line

tests/functional/lang/parse-fail-crlf.nix

@@ -0,0 +1 @@
+parse-okay-crlf.nix

tests/functional/lang/parse-fail-eol-2.flags

@@ -0,0 +1 @@
+--extra-deprecated-features cr-line-endings

tests/functional/lang/parse-fail-eol-3.flags

@@ -0,0 +1 @@
+--extra-deprecated-features cr-line-endings

tests/functional/lang/parse-okay-crlf.flags

@@ -0,0 +1 @@
+--extra-deprecated-features cr-line-endings