Bug 227836

Summary: sysutils/py-salt : update to 2018.3.0_2
Product: Ports & Packages Reporter: Christer Edwards <christer.edwards>
Component: Individual Port(s)Assignee: Martin Wilke <miwi>
Status: Closed Overcome By Events    
Severity: Affects Many People CC: admin, ari, christer.edwards, jhujhiti, john, mga, miwi, python, sunpoet, swills, woodsb02
Priority: Normal Keywords: needs-qa
Version: LatestFlags: koobs: maintainer-feedback? (christer.edwards)
Hardware: Any   
OS: Any   
See Also: https://github.com/saltstack/salt/issues/47006
https://github.com/saltstack/salt/pull/48195
https://github.com/saltstack/salt/pull/48636
Attachments:
Description Flags
patch
none
py27 testport QA
none
py36 testport QA
none
Temp. fix for PY36 (should be available upstream in 2018.3.3) none

Description Christer Edwards 2018-04-29 04:00:05 UTC
Created attachment 192890 [details]
patch

This patch addresses the marathon and fx2 grain failures caused by Salt importing opt-1.pyc files. Tracked upstream here: https://github.com/saltstack/salt/issues/47006

The patch disables bytecode compilation to workaround the issue. Master and minion tested to start and run successfully.
Comment 1 Christer Edwards 2018-04-29 04:01:10 UTC
Created attachment 192891 [details]
py27 testport QA
Comment 2 Christer Edwards 2018-04-29 04:01:27 UTC
Created attachment 192892 [details]
py36 testport QA
Comment 3 ari 2018-04-29 04:14:57 UTC
I'm not an expert in python at all, but I don't understand why this package has any compiled python files included. Is that common in python packages? It increases the time for when it first runs, but that's not really an issue with salt is it?
Comment 4 Erick Turnquist 2018-04-30 19:11:29 UTC
Yes, USES=python creates byte-compiled code for Python packages (or at least those using distutils) by default. Nonroot users would be unable to write these files themselves.
Comment 5 Christer Edwards 2018-05-03 23:03:27 UTC
Salt generally runs as root, so I don't expect that to be an issue.

This patch still generates the .pyc files, just not the -O1 optimizations.
Comment 6 Tilman Keskinoz freebsd_committer freebsd_triage 2018-06-13 14:39:06 UTC
*** Bug 227780 has been marked as a duplicate of this bug. ***
Comment 7 Sven Ruediger 2018-07-06 11:51:34 UTC
This patch has not yet been applied, but the 2018.3.1 was already added to the quarterly mirror.

The syspaths bug causes errors on many major modules (on py36-salt).

Can you add this to the current release? This patch works perfectly on systems with python3.6
Comment 8 ari 2018-07-06 12:34:29 UTC
Either the salt port needs to be rolled back to 2017 or this needs to be fixed urgently. Absolutely no one can use the FreeBSD salt port without this patch.
Comment 9 Kubilay Kocak freebsd_committer freebsd_triage 2018-07-31 23:40:19 UTC
@Christer 

I don't believe the proposed approach will be appropriate:

1) Wont these compiled files be created at first run, leaving leftovers after pkg-deletion? QA (poudriere) wouldn't pick these up as salt would not have run between installation/deinstallation.

2) Has upstream fixed this issue upstream since reporting? If so, can you point us to any other issue(s) and/or commit(s) ?

Also, please remember to set maintainer-approval flag to + on attachments for ports you maintain

Also noted from the upstream issue:

@jhujhiti says:

"Specifically, f_noext = f_noext.replace(BIN_PRE_EXT, '') is an attempt to ignore the extra bits in the filename of a bytecode file, but it only removes cpython-36, so we end up loading a module named fx2.opt-1."

Has there been an attempt to fix that section of the code to not load/match the optimized file? 

It appears that that is the root cause, and addressing that will be the correct, appropriate and permanent fix.
Comment 10 Kubilay Kocak freebsd_committer freebsd_triage 2018-07-31 23:58:51 UTC
Looks like there's an open PR [1], and a subsequently superseding PR [2] (by the original author of the commit identified by jhujhiti), that seeks to resolves the issue:

The former is a better in-the-meantime solution than disabling optimized bytecode generation, though the latter (PR) is preferred, as it is provided by an upstream author 

Maintainer to confirm it fixes the issue. If so, please:

- update the patch (obsoleting existing attachments)
- confirm QA with that patch
- set merge-quarterly flag to ? if quarterly branch is affected

[1] https://github.com/angeloudy/salt/commit/205abe209d5d8af6db2a619fa0c97de26f7b4256
[2] https://github.com/saltstack/salt/pull/48636
Comment 11 Sven Ruediger 2018-09-13 21:19:15 UTC
Created attachment 197080 [details]
Temp. fix for PY36 (should be available upstream in 2018.3.3)
Comment 12 Christer Edwards 2018-09-22 20:57:41 UTC
New patch (simpler) available here: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=231609