From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by finch.gentoo.org (Postfix) with ESMTPS id 3FCD715800A for ; Mon, 31 Jul 2023 05:08:23 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id EC519E0C52; Mon, 31 Jul 2023 05:08:19 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [IPv6:2001:470:ea4a:1:5054:ff:fec7:86e4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id EA9ABE0BA1 for ; Mon, 31 Jul 2023 05:08:18 +0000 (UTC) Message-ID: Subject: Re: [gentoo-dev] [PATCH 1/7] eclass/nuget.eclass: introduce new eclass From: =?UTF-8?Q?Micha=C5=82_G=C3=B3rny?= To: gentoo-dev@lists.gentoo.org Date: Mon, 31 Jul 2023 07:08:09 +0200 In-Reply-To: <78e35eff-3e7b-8458-e705-61032bb15c6e@gentoo.org> References: <20230730142647.872-1-xgqt@gentoo.org> <4766db0bfe4ba2b55d74034f83608b8461145808.camel@gentoo.org> <78e35eff-3e7b-8458-e705-61032bb15c6e@gentoo.org> Organization: Gentoo Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.48.4 Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-dev@lists.gentoo.org Reply-to: gentoo-dev@lists.gentoo.org X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply MIME-Version: 1.0 X-Archives-Salt: f13bea3f-ec71-437b-b2cf-7af174025dc9 X-Archives-Hash: 364d6a08ff9d5ad6132dd7af389bbb52 On Sun, 2023-07-30 at 22:01 +0200, Maciej Bar=C4=87 wrote: > > > +# @ECLASS_VARIABLE: NUGETS > > > +# @DEFAULT_UNSET > > > +# @PRE_INHERIT > > > +# @DESCRIPTION: > > > +# String containing all NuGet packages that need to be downloaded. > > > +# Used by "nuget_uris". > > > +# > > > +# Example: > > > +# @CODE > > > +# NUGETS=3D" > > > +# ImGui.NET-1.87.2 > > > +# Config.Net-4.19.0 > > > +# " > > > +# > > > +# inherit dotnet-pkg > > > +# > > > +# ... > > > +# > > > +# SRC_URI+=3D" $(nuget_uris) " > > > +# @CODE > > > + > > > +# @FUNCTION: nuget_uris > > > +# @USAGE: > > > +# @DESCRIPTION: > > > +# Generates the URIs to put in SRC_URI to help fetch dependencies. > > > +# If no arguments provided, uses the "NUGETS" variable. > >=20 > > Sounds like you're copying the old, horribly slow cargo.eclass API. > > Don't do that, unless you have to. Prefer variable-setting approach > > used by modern cargo.eclass. > >=20 >=20 > Please elaborate. >=20 > Maybe such mechanisms could be extracted into standalone eclass? I do=20 > not know cargo, rust nor how it is used in Gentoo. >=20 Calling subshells in global scope is a very bad idea, and it is completely unnecessary in the most common case when NUGETS are defined prior to inherit and URIs are inserted into top-level SRC_URI. Just look at cargo.eclass. > > > +# @FUNCTION: nuget_link > > > +# @USAGE: > > > +# @DESCRIPTION: > > > +# Link a specified NuGet package at "nuget-path" to the "NUGET_PACKA= GES" > > > +# directory. > > > +# > > > +# Example: > > > +# @CODE > > > +# nuget_link "${DISTDIR}"/pkg.0.nupkg > > > +# @CODE > > > +# > > > +# This function is used inside "dotnet-pkg_src_unpack" > > > +# from the "dotnet-pkg" eclass. > > > +nuget_link() { > > > + [[ ! "${1}" ]] && die "${FUNCNAME}: no nuget path given" > > > + > > > + mkdir -p "${NUGET_PACKAGES}" || die > > > + > > > + local nuget_name=3D"${1##*/}" > > > + > > > + if [[ -f "${NUGET_PACKAGES}"/${nuget_name} ]] ; then > > > + ewarn "${FUNCNAME}: ${nuget_name} already exists" > >=20 > > What does that mean? What is the user supposed to do about it? Is it > > normal? If it's normal, then why are you warning about it? If it isn'= t > > normal, then why isn't this fatal? >=20 > This can happen if NuGets copied from SYSTEM_NUGETS are overwritten by= =20 > ones specified in ebuild. >=20 > This might be desired by ebuild author. >=20 > We need to have this logged, I think ewarn is appropriate. ewarn is a warning for the end user. The end user doesn't benefit from being warned for "ebuild is doing something that might be wrong, or might be perfectly fine, ignore this". --=20 Best regards, Micha=C5=82 G=C3=B3rny