Created attachment 541948 [details] emerge-info.txt Portage reports the following QA issue: > * QA Notice: Global shell options changed and were not restored while calling 'src_prepare'
Created attachment 541950 [details] build.log.xz
portage's QA warning should be improved to perform diff as well. emerge --debug shows it's a mismatch in: -expand_aliases on +expand_aliases off
Seems to be triggered by epatch() call on a directory in toolchain.eclass:src_prepare(): EPATCH_MULTI_MSG="Applying Gentoo patches ..." \ epatch "${WORKDIR}"/patch
Does not seem to be gcc-specific problem (it's trigger $ cat sys-devel/bug/bug-0.ebuild # Copyright 1999-2018 Gentoo Foundation # Distributed under the terms of the GNU General Public License v2 EAPI="6" inherit estack RDEPEND="" DEPEND="" SLOT="0" src_unpack() { mkdir "${S}" eshopts_push -o noglob eshopts_pop } $ ebuild sys-devel/bug/bug-0.ebuild clean unpack >>> Unpacking source... * QA Notice: Global shell options changed and were not restored while calling 'src_unpack' Reassigning to base-system@ as estack.eclass maintainer.
I think 'expand_aliases' is set in portage's helpers via bin/isolated-functions.sh: shopt -s expand_aliases and does not get unset. I expect some portage like einfo/elog helper to set it at unexpected time.
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=e910f4f790f8da6d545f18cb1bc65abb3b60c81d commit e910f4f790f8da6d545f18cb1bc65abb3b60c81d Author: Sergei Trofimovich <slyfox@gentoo.org> AuthorDate: 2018-08-08 08:07:00 +0000 Commit: Sergei Trofimovich <slyfox@gentoo.org> CommitDate: 2018-08-08 08:07:10 +0000 sys-devel/gcc: use 8.2.0 patchset for 8.2.0 release No change to gcc source tree. As EPATCH_EXCLUDE is not used anymore this change happens to workaround spurious QA warning in bug #662586. Bug: https://bugs.gentoo.org/662586 Package-Manager: Portage-2.3.44, Repoman-2.3.10 sys-devel/gcc/Manifest | 1 + sys-devel/gcc/gcc-8.2.0.ebuild | 10 +--------- 2 files changed, 2 insertions(+), 9 deletions(-)
(In reply to Sergei Trofimovich from comment #5) > I think 'expand_aliases' is set in portage's helpers via > bin/isolated-functions.sh: > shopt -s expand_aliases > and does not get unset. I expect some portage like einfo/elog helper to set > it at unexpected time. Simpler reproducer: src_unpack() { local save=$(shopt -p -o) shopt -p > 1 set -o noglob shopt -p > 2 diff -dup 1 2 eval "${save}" shopt -p > 3 diff -dup 1 3 } The eval call is causing the change in expand_aliases. Looks like it's either a bug in bash or a feature.
By the way, the QA warning is not spurious. The only spurious thing here is your arrogance which makes you commit broken stuff because you're confident it's not your code being broken. You can read [1] as how to mangle options correctly, without undesirable side effects. [1]:https://blogs.gentoo.org/mgorny/2016/01/25/mangling-shell-options-in-ebuilds/
If it helps, I suppose we could eliminate the expand_aliases usage from portage. Currently portage only has two alias, save_IFS and restore_IFS, and they could easily be inlined since they are only used in one place.
(In reply to Michał Górny from comment #8) > By the way, the QA warning is not spurious. Totally could be. > The only spurious thing here is > your arrogance which makes you commit broken stuff because you're confident > it's not your code being broken. I suggest to continue in bug #663192 on this topic as it does not belong here.
I see that you really fail to notice what you've done. You've left a *permanent false statement* about the QA warning being spurious in the git log where it was completely unnecessary and which does not provide any way to correct that statement.
(In reply to Michał Górny from comment #11) > I see that you really fail to notice what you've done. Yes. I can't see a point if it's not stated directly and clearly. > You've left a > *permanent false statement* about the QA warning being spurious I admit it was not a correct statement. People make mistakes and I do a _lot_ of them. If it's any consolation, commit message has a bug link. Here the bug should be explained clearer in comments and/or have an URL field to point to an authoritative doc (say, wiki) explaining the subtleties. I have no 100% confidence in anything related to software and don't have that expectation from anyone else. All my statements should be taken with a grain of salt. > in the git log where it was completely unnecessary You did not say _why_ in your opinion commit wording: 1. is technically problematic: does it confuse you when you read it? Can it trick someone into applying incorrect fix to other packages? (Something else?) 2. is personally problematic to the point that it required an insult: did it attack you? If it was it unintentional. I don't enjoy being attacked myself (and was not able to think clearly for a day after all this) > and which does not provide any way to correct that statement. I don't understand this part. Is it about git history being immutable or something completely different? Does not bug link do? If "why" part warrants the change I can do a git revert/git apply with amended message. Please provide your wording for the change for this case so I can evaluate the difference as sys-devel/gcc maintainer.
First of all, I'm sorry, I've misread your commit message. After reading it a few times now, it finally got to me that you've made the warning disappear and not caused it. So yes, the commit message is confusing a lot. My point was that somebody having the same (or very similar issue) could -- upon reading this commit message -- wrongly assume the warning is spurious and ignore it. And yes, git history is immutable. That's why it is important to try to have good commit messages.
Got it. Thanks!
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=b21e386a064e1ed49981d9527f968ddc5e1eeb86 commit b21e386a064e1ed49981d9527f968ddc5e1eeb86 Author: Sergei Trofimovich <slyfox@gentoo.org> AuthorDate: 2018-08-10 23:46:23 +0000 Commit: Sergei Trofimovich <slyfox@gentoo.org> CommitDate: 2018-08-18 23:28:04 +0000 epatch.eclass: drop 'estack.eclass' usage Avoid use of eshopts_push / eshopts_pop functions as they don't preserve expand_aliases shell option and get detected by QA warning as: * QA Notice: Global shell options changed and were not restored while calling 'src_prepare' Ssee bug #662586 for details. Tested as: $ EPATCH_USER_EXCLUDE="*" ebuild gcc-8.2.0.ebuild clean prepare Bug: https://bugs.gentoo.org/662586 Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org> eclass/epatch.eclass | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
*** Bug 638152 has been marked as a duplicate of this bug. ***
*** Bug 670318 has been marked as a duplicate of this bug. ***
*** Bug 670398 has been marked as a duplicate of this bug. ***
*** Bug 670404 has been marked as a duplicate of this bug. ***
*** Bug 670414 has been marked as a duplicate of this bug. ***
*** Bug 670426 has been marked as a duplicate of this bug. ***
*** Bug 670432 has been marked as a duplicate of this bug. ***
*** Bug 670442 has been marked as a duplicate of this bug. ***
*** Bug 670532 has been marked as a duplicate of this bug. ***
*** Bug 670544 has been marked as a duplicate of this bug. ***
Reported as Bash bug: https://lists.gnu.org/archive/html/bug-bash/2019-11/msg00092.html
The bug has been closed via the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=d2cb9490dbee48a32f196d1aa80d7356a99d9fd8 commit d2cb9490dbee48a32f196d1aa80d7356a99d9fd8 Author: Ulrich Müller <ulm@gentoo.org> AuthorDate: 2019-11-23 10:18:18 +0000 Commit: Ulrich Müller <ulm@gentoo.org> CommitDate: 2019-11-26 19:23:19 +0000 estack.eclass: Properly restore shopt options. Calling "eshopts_push; eshopts_pop" makes Portage report a QA issue: * QA Notice: Global shell options changed and were not restored while calling 'src_prepare' This is caused by some side effect in bash, by which disabling the "posix" option (even if it was already disabled before) in a non-interactive shell also disables the "expand_aliases" option. Work around the problem by always saving and restoring both "set -o" and "shopt" option sets. Also fix "estack_push -s" which should not execute shopt when called without further parameters. Closes: https://bugs.gentoo.org/662586 Signed-off-by: Ulrich Müller <ulm@gentoo.org> eclass/estack.eclass | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
(In reply to Ulrich Müller from comment #26) > Reported as Bash bug: > https://lists.gnu.org/archive/html/bug-bash/2019-11/msg00092.html This is fixed in bash-5.1.