grass @8.2.1: SyntaxError: invalid syntax

[ryandesign (Ryan Carsten Schmidt)]

/opt/local/bin/grass is syntactically invalid, as shown by this failed build of gdal-grass which depends on grass:

​https://build.macports.org/builders/ports-10.13_x86_64-builder/builds/193782/steps/install-port/logs/stdio

Error: Failed to configure gdal-grass:   File "/opt/local/bin/grass", line 107
    MACOS = sys.platform.startswith("darwin")nos.environ["GRASS_PYTHON"] = "/opt/local/bin/python3.10"
                                             ^^^
SyntaxError: invalid syntax
DEBUG: Error code: NONE

I think the problem was introduced in [fb098cb6c9a5df0e16d138b692fdf81abdf61e27/macports-ports]:

​https://github.com/macports/macports-ports/blob/1dc28c18e4e00b033b04bd0b710c129c36be7043/gis/grass/Portfile#L132-L133

[nilason (Nicklas Larsson)]

I assume you meant grass @8.2.1 (8.0.1 would suggest the obsolete grass8 port).

The reinplace works correctly for me locally, the ...nos.environ... part is supposed to be a line break ('\n') following with 'os.environ...':

MACOS = sys.platform.startswith("darwin")
os.environ["GRASS_PYTHON"] = "/opt/local/bin/python3.10"

Judging by the bot builds' logs, the reinplace fails to produce the intended result on pre-11 macOS versions. I have no idea what could be the reason to this.

An alternative to the line break, could be the use of a ';'.

[nilason (Nicklas Larsson)]

An alternative to the line break, could be the use of a ';'.

This is actually what I suggest in the PR ​https://github.com/macports/macports-ports/pull/18898.

(Even though I'm curious on the cause of the 'reinplace' failure on some systems).

[ryandesign (Ryan Carsten Schmidt)]

Replying to nilason:

I assume you meant grass @8.2.1 (8.0.1 would suggest the obsolete grass8 port).

Yes, sorry, I was looking at the version in [fb098cb6c9a5df0e16d138b692fdf81abdf61e27/macports-ports].

Judging by the bot builds' logs, the reinplace fails to produce the intended result on pre-11 macOS versions. I have no idea what could be the reason to this.

I guess the ability to use \n to mean a newline in the replacement pattern is not something that was present in the version of sed that shipped before macOS 11:

% sw_vers -productVersion
10.15.7
% echo hello | sed 's|hello|&\nworld|'               
hellonworld
%

A workaround is to literally insert a newline in the replacement string, rather than an n:

% echo hello | sed 's|hello|&\
world|'
hello
world
%

[ryandesign (Ryan Carsten Schmidt)]

We typically don't do these kinds of complex replacements with reinplace because something as simple as a version update can cause them to no longer work due to minor changes in upstream code, and because MacPorts only warns and does not error when this happens. MacPorts developers often ignore these warnings (or perhaps do not see them because they are buried in a flood other output in debug or verbose modes) with the consequence that changes that a Portfile was intending to make are no longer being made. This might lead to wrong behavior of the port, which users would then discover and hopefully report, but probably not before being annoyed that they had to take time out of their day to do that.

Instead, for anything more complex than replacing one value with another, we write a patchfile. The patchfile contains a placeholder, such as @PYBIN@ in this case. The Portfile applies the patch and then afterward uses reinplace to replace the @PYBIN@ placeholder with the correct Portfile variable. If the patchfile ever fails to apply in some future version of the software, MacPorts will error, forcing the MacPorts developer to analyze why and either update the patchfile or remove it and the reinplace. The context provided by the patchfile can assist the developer in determining what the code looked like before and figuring out whether similar code that still needs patching exists in the file now.

[nilason (Nicklas Larsson)]

Replying to ryandesign:

We typically don't do these kinds of complex replacements with reinplace because something as simple as a version update can cause them to no longer work due to minor changes in upstream code, and because MacPorts only warns and does not error when this happens. MacPorts developers often ignore these warnings (or perhaps do not see them because they are buried in a flood other output in debug or verbose modes) with the consequence that changes that a Portfile was intending to make are no longer being made. This might lead to wrong behavior of the port, which users would then discover and hopefully report, but probably not before being annoyed that they had to take time out of their day to do that.

Instead, for anything more complex than replacing one value with another, we write a patchfile. The patchfile contains a placeholder, such as @PYBIN@ in this case. The Portfile applies the patch and then afterward uses reinplace to replace the @PYBIN@ placeholder with the correct Portfile variable. If the patchfile ever fails to apply in some future version of the software, MacPorts will error, forcing the MacPorts developer to analyze why and either update the patchfile or remove it and the reinplace. The context provided by the patchfile can assist the developer in determining what the code looked like before and figuring out whether similar code that still needs patching exists in the file now.

Again, thanks for the explanation! You convinced me to use patch files also in these cases, updated the PR accordingly.

(Note to self: I'd better address this upstreams though).

[nilason (Nicklas Larsson)]

In 0d33fb3b010274869e89b5c521e096a38513672a/macports-ports (master):

grass: fix bad reinplace on pre-11 macOS builds

Closes #67537

In addition, remove unnecessary 'platforms darwin' from the port gdal-grass.