Skip to content
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

Jump to bottom

dev-lang/erlang-21.1 #9357

Closed
wants to merge 1 commit into from
Closed

dev-lang/erlang-21.1 #9357

wants to merge 1 commit into from

Conversation

stefantalpalaru
Copy link
Contributor

I removed some deprecated configure options, properly (re-)added the "wxwidgets" USE flag - without automagic dependencies, added the "pgo" flag and made some USE flags enabled by default to match upstream's defaults.

Closes: https://bugs.gentoo.org/658656 and https://bugs.gentoo.org/653700

@gentoo-bot gentoo-bot added maintainer-needed There is at least one affected package with no maintainer. Review it if you can. assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits. labels Jul 26, 2018
stefantalpalaru added a commit to stefantalpalaru/gentoo that referenced this pull request Jul 26, 2018
@stefantalpalaru
media-gfx/wings-2.1.7-r1
125efb8 
the "smp" USE flag is no longer available in dev-lang/erlang-21.0.x
related PR: gentoo#9357
@stefantalpalaru stefantalpalaru mentioned this pull request Jul 26, 2018
Closed
@ghost
Copy link

ghost commented Jul 29, 2018 •
edited by ghost

It's worth clarifying the context explicitly here to avoid leaving this PR go unattended without explanation.

This package has no maintainer and has some complexity associated with it. There is little chances that a gentoo developer will do a "drive by" review of this unmaintained package. It means that it either needs a gentoo developer taking over or a proxy maintainer.

That boils down to: @stefantalpalaru are you willing to take over the maintenance of this package? It implies adhering to Gentoo's consensus regarding coding style and best practices. Seeing how things go in #9360 , I have the feeling that it's not something that you're willing to do, so I prefer to ask explicitly to avoid wasting everyone's time.

@stefantalpalaru
Copy link
Contributor Author

@stefantalpalaru are you willing to take over the maintenance of this package?

Yes, of course. How many times do I have to agree to being a proxy maintainer? This must be the fifth time, already.

Seeing how things go in #9360 , I have the feeling that it's not something that you're willing to do, so I prefer to ask explicitly to avoid wasting everyone's time.

Seeing how I added the "Proxy Maintainers" project to metadata.xml, it's rather obvious I intend to proxy-maintain the package. What's with all these hostile "feelings"?

@ghost
Copy link

ghost commented Jul 29, 2018

I ask explicitly because from what I read in #9360, it seems that you didn't read https://wiki.gentoo.org/wiki/Project:Proxy_Maintainers#What_it_takes_to_be_a_Proxied_Maintainer before agreeing to become a proxy maintainer.

If you tell us that you read and understood this section, then allright, no problem, it means that #9360 will be resolved to everyone satisfaction with you making requested changes so we can then confidently invest time into reviewing this PR here.

@stefantalpalaru
Copy link
Contributor Author

If you tell us that you read and understood this section, then allright, no problem

Virgil, this is getting absurd. Grow a pair and close all my pull requests instead of looking for silly excuses.

it means that #9360 will be resolved to everyone satisfaction with you making requested changes so we can then confidently invest time into reviewing this PR here

You people still haven't explained how exactly you merge GitHub pull request. Since I don't see commits that are not from Gentoo developers in the main Git repo, it logically follows that you do not pull and merge external commits, so asking me to squash, mark or otherwise apply your own commit guideline to PR commits is an absurd request - yet another absurd requests from people looking for excuses to reject external contributions...

As for your precious time, you seem to have plenty when it comes to arguing with Gentoo users, and none for maintaining Gentoo packages.

@mgorny
Copy link
Member

mgorny commented Jul 29, 2018

@stefantalpalaru please cut this. I won't tolerate attacking people who are trying to help. This is your last and only warning.

@ghost
Copy link

ghost commented Jul 29, 2018

Nobody wishes to close your pull requests. This package is obviously in need of a maintainer, we welcome help. Conditions for acceptance of that help are clear and you apparently decide that you don't like those conditions. Fine.

I think that every gentoo developers involved in your recent PRs has shown good faith. We do, however, protect ourselves from wasting our time because it does seem, from the tone of your messages, that you came here with the intention of being rejected and continue your crusade against gentoo developers saying "look! I was rejected!". Why would we waste time reviewing work from someone who shows no intention of carrying on?

If that's what you want to do, being rejected, fine, but you will be the worst off for it because we will have shown good faith. If I haven't mischaracterised your intent, then the "ballsy" thing to do for you would be to close your PRs yourself.

mgorny
mgorny requested changes Aug 15, 2018
View reviewed changes
Copy link
Member

@mgorny mgorny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please split adding new version from removing old. Also Closes tag accepts a single argument only, so you need to split it.

dev-lang/erlang/erlang-21.0.4.ebuild Outdated
emacs? ( virtual/emacs )
java? ( >=virtual/jdk-1.2:* )
odbc? ( dev-db/unixODBC )
sys-libs/ncurses:*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ncurses:* is almost certainly wrong as slot :5 does not provide headers to build on. Also, please move unconditional deps before USE conditionals.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

dev-lang/erlang/erlang-21.0.4.ebuild Outdated

RDEPEND="
emacs? ( virtual/emacs )
java? ( >=virtual/jdk-1.2:* )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the explicit version here. 1.2 is gone for years now, and I seriously doubt you've actually tested it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

dev-lang/erlang/erlang-21.0.4.ebuild Outdated
$(use_with ssl) \
$(use_with ssl ssl-rpath "no") \
$(use_enable ssl dynamic-ssl-lib) \
$(use_enable systemd) \
$(use_enable pgo) \
$(if ! use wxwidgets; then echo "--with-wxdir=/dev/null"; fi) \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use usex, please.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

dev-lang/erlang/erlang-21.0.4.ebuild Outdated
else
dosym "${ERL_LIBDIR}/erts-${ERL_ERTS_VER}/bin/beam" /usr/bin/beam
fi
dosym "${ERL_LIBDIR}/erts-${ERL_ERTS_VER}/bin/beam.smp" /usr/bin/beam.smp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use relative symlinks instead of absolute everywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@stefantalpalaru stefantalpalaru changed the title dev-lang/erlang-21.0.4 dev-lang/erlang-21.0.4 [please reassign] Aug 15, 2018
@gentoo-bot gentoo-bot changed the title dev-lang/erlang-21.0.4 [please reassign] dev-lang/erlang-21.0.4 Aug 15, 2018
@gentoo-bot
Copy link

Pull Request assignment

Areas affected: ebuilds
Packages affected: dev-lang/erlang

dev-lang/erlang: @gentoo/proxy-maint (maintainer needed)

Bugs linked: 658656, 653700

In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

@gentoo-bot gentoo-bot added maintainer-needed There is at least one affected package with no maintainer. Review it if you can. assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. and removed assigned PR successfully assigned to the package maintainer(s). maintainer-needed There is at least one affected package with no maintainer. Review it if you can. no bug found No Bug/Closes found in the commits. labels Aug 15, 2018
@stefantalpalaru
Copy link
Contributor Author

Please split adding new version from removing old.

Since my commits don't end up in the main tree, feel free to split this PR any way you want when you create your own commits.

@mgorny
Copy link
Member

mgorny commented Aug 15, 2018

The combined removal+addition makes it impossible to leave comments on ebuild parts not covered by the change. Please fix the pull request, so that I can review it properly.

Also, please fix the merge conflicts.

@stefantalpalaru stefantalpalaru changed the title dev-lang/erlang-21.0.4 dev-lang/erlang-21.0.5 Aug 15, 2018
@stefantalpalaru
Copy link
Contributor Author

Also, please fix the merge conflicts.

Done. It was just the trivial matter of merging "master" into this PR branch.

The combined removal+addition makes it impossible to leave comments on ebuild parts not covered by the change. Please fix the pull request, so that I can review it properly.

You're asking me to fix GitHub. The PR is now for erlang-21.0.5 because it's been 3 weeks since it was created.
See if merging "master" changed anything for the better in the GitHub PR commenting department.

@mgorny
Copy link
Member

mgorny commented Aug 15, 2018

No, I'm asking you to stop removing the old version. If you can't do that, I don't think there's anything else to do for us here.

@stefantalpalaru
Copy link
Contributor Author

"erlang-21.0.2.ebuild" brought back from "master". Do keep in mind that the more useless versions you have for a package, the harder it is to maintain it.

And Michał, you need to start acting like an adult. You lashed out with a Kafkaesque reversal in "I won't tolerate attacking people who are trying to help", continued with a ridiculously aggressive threat - "This is your last and only warning." - and now you stomp your feet like a child instead of clarifying what it is that you need ("If you can't do that, I don't think there's anything else to do for us here." - more dramatic than a primadonna ;-)).

I can fix technical issues, but I can't fix your power trips, so keep that in mind the next time you lash out.

a17r
a17r previously requested changes Aug 15, 2018
View reviewed changes
Copy link
Member

@a17r a17r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see 5 commits where there should be one.

mgorny
mgorny reviewed Aug 15, 2018
View reviewed changes
dev-lang/erlang/erlang-21.0.5.ebuild Outdated
# NOTE: If you need symlinks for binaries please tell maintainers or
# open up a bug to let it be created.

UPSTREAM_V="$(get_version_component_range 1-2)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

versionator.eclass is deprecated, please switch to eapi7-ver.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

dev-lang/erlang/erlang-21.0.5.ebuild Outdated
DESCRIPTION="Erlang programming language, runtime environment and libraries (OTP)"
HOMEPAGE="http://www.erlang.org/"
SRC_URI="https://github.com/erlang/otp/archive/OTP-${PV}.tar.gz -> ${P}.tar.gz
http://erlang.org/download/otp_doc_man_${UPSTREAM_V}.tar.gz -> ${PN}_doc_man_${UPSTREAM_V}.tar.gz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switch to https, if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

dev-lang/erlang/erlang-21.0.5.ebuild Outdated
LICENSE="Apache-2.0"
SLOT="0"
KEYWORDS="~alpha ~amd64 ~arm ~arm64 ~hppa ~ia64 ~ppc ~ppc64 ~sparc ~x86 ~x86-fbsd ~amd64-linux ~x86-linux ~ppc-macos ~x64-macos ~x86-macos ~x64-solaris"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the unnecessary empty line here to match the standard ebuild template.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

dev-lang/erlang/erlang-21.0.5.ebuild Outdated
S="${WORKDIR}/otp-OTP-${PV}"

PATCHES=(
"${FILESDIR}/18.2.1-wx3.0.patch"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, single tab for indent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

dev-lang/erlang/erlang-21.0.5.ebuild Outdated

# Taken from the upstream build script, build_otp
erlang_create_lib_configure_in() {
local bootstrap_apps="erts lib/asn1 lib/compiler lib/hipelib/ic lib/kernel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a bash array.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

dev-lang/erlang/erlang-21.0.5.ebuild Outdated
}

extract_version() {
sed -n -e "/^$2 = \(.*\)$/s::\1:p" "${S}/$1/vsn.mk"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

||die. Also, would be nice to document the parameters, at least by assigning them to local vars with clear names.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

dev-lang/erlang/erlang-21.0.5.ebuild Outdated
emake INSTALL_PREFIX="${ED}" install

if use doc ; then
local DOCS=( "AUTHORS" "HOWTO"/* "README.md" "CONTRIBUTING.md" "${WORKDIR}"/doc/. "${WORKDIR}"/lib/. "${WORKDIR}"/erts-* )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, with USE=-doc you use automagic file list, with uSE=doc you redefine the whole list. This is just asking for inconsistency. Maybe just call einstalldocs first, then use dodoc -r to install extra files manually.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DOCS is now set explicitly with USE="-doc".

dev-lang/erlang/erlang-21.0.5.ebuild Outdated
local ERL_LIBDIR_REL="$(get_libdir)/erlang"
local ERL_INTERFACE_VER="$(extract_version lib/erl_interface EI_VSN)"
local ERL_ERTS_VER="$(extract_version erts VSN)"
local MY_MANPATH="/usr/share/${PN}/man"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, why do you need this variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my_manpath is used twice

dev-lang/erlang/erlang-21.0.5.ebuild Outdated
# extend MANPATH, so the normal man command can find it
# see bug 189639
echo "MANPATH=\"${MY_MANPATH}\"" > "${T}/90erlang" || die
doenvd "${T}/90erlang"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use newenvd - ... and input the data via stdin, to avoid temporary file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

dev-lang/erlang/erlang-21.0.5.ebuild Outdated
doenvd "${T}/90erlang"

if use emacs ; then
pushd "${S}" &>/dev/null || die
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused. Why wouldn't you be in ${S} at this point?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

stefantalpalaru added a commit to stefantalpalaru/gentoo-overlay that referenced this pull request Aug 15, 2018
@stefantalpalaru
https://github.com/gentoo/gentoo/pull/9357#pullrequestreview-146594792
20adf02
stefantalpalaru added a commit to stefantalpalaru/gentoo that referenced this pull request Aug 25, 2018
@stefantalpalaru
media-gfx/wings: dependency default for the "smp" USE flag
40381db 
The "smp" USE flag is no longer available in dev-lang/erlang-21.0.x
Related PR: gentoo#9357

Authored-by: Ștefan Talpalaru <stefantalpalaru@yahoo.com>
Closes: gentoo#9358
@stefantalpalaru stefantalpalaru changed the title dev-lang/erlang-21.0.6 dev-lang/erlang-21.0.9 Sep 22, 2018
mgorny
mgorny requested changes Sep 23, 2018
View reviewed changes
Copy link
Member

@mgorny mgorny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

  dependency.badindev           6
   dev-lang/erlang/erlang-21.0.9.ebuild: DEPEND: ~arm64(default/linux/arm64/17.0)
['x11-libs/wxGTK:3.0-gtk3[X,opengl]']
   dev-lang/erlang/erlang-21.0.9.ebuild: RDEPEND: ~arm64(default/linux/arm64/17.0)
['x11-libs/wxGTK:3.0-gtk3[X,opengl]']
   dev-lang/erlang/erlang-21.0.9.ebuild: DEPEND: ~arm64(default/linux/arm64/17.0/desktop/systemd)
['x11-libs/wxGTK:3.0-gtk3[X,opengl]']
   dev-lang/erlang/erlang-21.0.9.ebuild: RDEPEND: ~arm64(default/linux/arm64/17.0/desktop/systemd)
['x11-libs/wxGTK:3.0-gtk3[X,opengl]']
   dev-lang/erlang/erlang-21.0.9.ebuild: DEPEND: ~arm64(default/linux/arm64/17.0/systemd)
['x11-libs/wxGTK:3.0-gtk3[X,opengl]']
   dev-lang/erlang/erlang-21.0.9.ebuild: RDEPEND: ~arm64(default/linux/arm64/17.0/systemd)
['x11-libs/wxGTK:3.0-gtk3[X,opengl]']

@mgorny
Copy link
Member

mgorny commented Sep 23, 2018

Also it fails to build:
dev-lang:erlang-21.0.9:20180923-075447.log

@stefantalpalaru
Copy link
Contributor Author

wxGTK:3.0-gtk3 is not keyworded on arm64, so we're back to using the GTK+2-based wxGTK (there's probably no technical impediment, since gtk+:3 is available on that platform).

You had a problem with wxWidgets detection, which is now fixed by passing "--with-wx-config=${WX_CONFIG}" to ./configure.

For your PGO problem, just disable ccache and distcc, as you always do when there's some profiling going on.

@mgorny
Copy link
Member

mgorny commented Sep 23, 2018

wxGTK:3.0-gtk3 is not keyworded on arm64, so we're back to using the GTK+2-based wxGTK (there's probably no technical impediment, since gtk+:3 is available on that platform).

Mask the flag on arm64 profile, and request keywording.

For your PGO problem, just disable ccache and distcc, as you always do when there's some profiling going on.

ccache/distcc is a supported Gentoo configuration, and this particular setup is guaranteed-safe. If anything's broken because of it, then it means the package is seriously broken and needs to be fixed.

@stefantalpalaru
Copy link
Contributor Author

Mask the flag on arm64 profile, and request keywording.

I'd rather not go on yet another wild-goose chase.

ccache/distcc is a supported Gentoo configuration, and this particular setup is guaranteed-safe. If anything's broken because of it, then it means the package is seriously broken and needs to be fixed.

FEATURES="ccache distcc" works for me, with a local distccd server, so it's probably your setup that's broken.

Can you build the erlang-21.0.9 that's in the main tree right now? That one also has PGO enabled, with no way to disable it.

@mgorny
Copy link
Member

mgorny commented Sep 24, 2018

I'd rather not go on yet another wild-goose chase.

That's how we do things on Gentoo.

Can you build the erlang-21.0.9 that's in the main tree right now? That one also has PGO enabled, with no way to disable it.

Fails the same. Still, if I can't test it, I can't merge it.

@mgorny
Copy link
Member

mgorny commented Sep 24, 2018

So it's not 'my system being broken' as you wished it were but a parallel make issue. Fix it.

@stefantalpalaru
Copy link
Contributor Author

a parallel make issue

I can't be. I have MAKEOPTS="-j9 -l9".

@mgorny
Copy link
Member

mgorny commented Sep 24, 2018

And I have -j12 without load restrictions, and in case you don't know, parallel make issues exhibit themselves randomly.

@stefantalpalaru
Copy link
Contributor Author

parallel make issues exhibit themselves randomly

If you always experience the problem and I can never replicate it, it's not random.

Do you use some configure cache that's also used outside Gentoo? This is from your log:

checking for a BSD-compatible install... (cached) /usr/bin/install -c

and this is from mine:

checking for a BSD-compatible install... /usr/lib/portage/python2.7/ebuild-helpers/xattr/install -c

@mgorny
Copy link
Member

mgorny commented Sep 24, 2018

Yes, I use a site cache to speed up the few repeating checks:

ac_cv_header_dlfcn_h=${ac_cv_header_dlfcn_h=yes}
ac_cv_header_inttypes_h=${ac_cv_header_inttypes_h=yes}
ac_cv_header_memory_h=${ac_cv_header_memory_h=yes}
ac_cv_header_stdc=${ac_cv_header_stdc=yes}
ac_cv_header_stdint_h=${ac_cv_header_stdint_h=yes}
ac_cv_header_stdlib_h=${ac_cv_header_stdlib_h=yes}
ac_cv_header_string_h=${ac_cv_header_string_h=yes}
ac_cv_header_strings_h=${ac_cv_header_strings_h=yes}
ac_cv_header_sys_stat_h=${ac_cv_header_sys_stat_h=yes}
ac_cv_header_sys_types_h=${ac_cv_header_sys_types_h=yes}
ac_cv_header_unistd_h=${ac_cv_header_unistd_h=yes}
ac_cv_path_EGREP=${ac_cv_path_EGREP='/bin/grep -E'}
ac_cv_path_FGREP=${ac_cv_path_FGREP='/bin/grep -F'}
ac_cv_path_GREP=${ac_cv_path_GREP=/bin/grep}
ac_cv_path_SED=${ac_cv_path_SED=/bin/sed}
ac_cv_path_install=${ac_cv_path_install='/usr/bin/install -c'}
ac_cv_path_mkdir=${ac_cv_path_mkdir=/bin/mkdir}
ac_cv_prog_AWK=${ac_cv_prog_AWK=gawk}
ac_cv_prog_make_make_set=${ac_cv_prog_make_make_set=yes}
lt_cv_file_magic_cmd=${lt_cv_file_magic_cmd='$MAGIC_CMD'}
lt_cv_file_magic_test_file=${lt_cv_file_magic_test_file=}
lt_cv_sharedlib_from_linklib_cmd=${lt_cv_sharedlib_from_linklib_cmd='printf %s\n'}
lt_cv_sys_max_cmd_len=${lt_cv_sys_max_cmd_len=1572864}

stefantalpalaru added a commit to stefantalpalaru/gentoo that referenced this pull request Sep 25, 2018
@stefantalpalaru
media-gfx/wings: dependency default for the "smp" USE flag
9b6879b 
The "smp" USE flag is no longer available in dev-lang/erlang-21.0.x
Related PR: gentoo#9357

Closes: gentoo#9358
Authored-by: Ștefan Talpalaru <stefantalpalaru@yahoo.com>
Signed-off-by: Ștefan Talpalaru <stefantalpalaru@yahoo.com>
@stefantalpalaru
dev-lang/erlang: version bump to 21.1
0733b20 
I removed some deprecated configure options, properly (re-)added the
"wxwidgets" USE flag - without automagic dependencies, added the "pgo"
flag and made some USE flags enabled by default to match upstream's
defaults.

Most of the ebuild was refactored, after code reviews, with the more
significant change being the replacement of two shell functions copied
from an upstream build script with a call to a patched version of that
script.

When the "pgo" USE flag is enabled, distcc and ccache are disabled, to
avoid a build failure due to a version mismatch in .gcda files that
cannot be explained by different GCC versions.

Closes: https://bugs.gentoo.org/653700
Closes: https://bugs.gentoo.org/658656
Authored-by: Ștefan Talpalaru <stefantalpalaru@yahoo.com>
Signed-off-by: Ștefan Talpalaru <stefantalpalaru@yahoo.com>
@stefantalpalaru stefantalpalaru changed the title dev-lang/erlang-21.0.9 dev-lang/erlang-21.1 Sep 28, 2018
@stefantalpalaru
Copy link
Contributor Author

When the "pgo" USE flag is enabled, distcc and ccache are disabled, to
avoid a build failure due to a version mismatch in .gcda files that
cannot be explained by different GCC versions.

While working on this, I discovered that the "export DISTCC_DISABLE=1" line - found in 13 ebuilds in the main tree - doesn't work at all with the latest distcc and it probably never worked. Looks like someone copied the (working) "export CCACHE_DISABLE=1" line without testing it, then stuck a "try to turn off distcc and ccache for people that have a problem with it" comment :-)

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2018-09-28 17:37 UTC
Newest commit scanned: 0733b20
Status: ✅ good

Issues already there before the PR (double-check them):
https://qa-reports.gentoo.org/output/gentoo-ci/2ffe449/output.html#virtual/linux-sources

@mgorny
Copy link
Member

mgorny commented Sep 28, 2018

I use versioned CC/CXX, so version conflict can't happen in my case.

@stefantalpalaru
Copy link
Contributor Author

The good news is that I identified the problem. The bad news is that we can't do profile-guided optimisations over distcc.

The Erlang build system wants to copy .gcda files, making the assumption that it will find them in the same directory and with the same prefix as the corresponding object files (default behaviour when compiling locally).

What happens under distcc is that something like this is produced instead during a profiling run: "/tmp/distccd_dfedc28b.gcda" because the original source file was renamed to "/tmp/distccd_dfedc28b.c" remotely.

It's possible to manipulate that path, somewhat, but not the filename: https://gcc.gnu.org/onlinedocs/gcc/Cross-profiling.html

Further more, it's not possible to pass the .gcda file created by the profiling run to distcc for the second compilation step and without that profile there is no optimisation being done. In gcc-8.2.0 that little problem flies under the radar, but a few days ago they added a specific warning flag for that in the trunk: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86957

So unless you want to make significant changes to distcc and gcc, we have to always disable distcc when we're doing PGO. This applies to all packages with a "pgo" USE flag (except maybe gcc that uses the previously built compiler in its second and third phase, thus skipping distcc).

P.S.: that "version mismatch in .gcda files" I was talking about in a previous comment was due to this: https://bugs.gentoo.org/667252

@mgorny
Copy link
Member

mgorny commented Sep 30, 2018

P.S.: that "version mismatch in .gcda files" I was talking about in a previous comment was due to this: https://bugs.gentoo.org/667252

This only applies if you're using bad distcc setup where CC/CXX does not contain compiler version.

@stefantalpalaru
Copy link
Contributor Author

stefantalpalaru commented Sep 30, 2018 •
edited

This only applies if you're using bad distcc setup where CC/CXX does not contain compiler version.

"Bad" setup or not, it's the default setup. Anyway, there were more factors leading to that mismatch.

@mgorny
Copy link
Member

mgorny commented Sep 30, 2018

It built now, I'll merge.

@stefantalpalaru
Copy link
Contributor Author

In the future, distcc will know not to distribute compilations involved in PGO: distcc/distcc#309

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgorny mgorny mgorny requested changes

@a17r a17r a17r left review comments

Assignees
No one assigned
Labels
assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. maintainer-needed There is at least one affected package with no maintainer. Review it if you can.
Projects
None yet
Milestone
No milestone
6 participants
@stefantalpalaru @mgorny @gentoo-bot @a17r @ryao @gentoo-repo-qa-bot