From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCHv4 3/5] Makefiles: Add experimental tag check and warnings to trigger on use Date: Mon, 22 Jan 2018 02:37:11 +0100 Message-ID: <2637029.nLuUIcJZsc@xps> References: <20171201185628.16261-1-nhorman@tuxdriver.com> <2231669.8C7kkI4IjB@xps> <20180122013417.GB5415@neilslaptop.think-freely.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org, Ferruh Yigit , john.mcnamara@intel.com, bruce.richardson@intel.com To: Neil Horman Return-path: Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id 60028A48A for ; Mon, 22 Jan 2018 02:37:49 +0100 (CET) In-Reply-To: <20180122013417.GB5415@neilslaptop.think-freely.org> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 22/01/2018 02:34, Neil Horman: > On Sun, Jan 21, 2018 at 07:54:44PM +0100, Thomas Monjalon wrote: > > 12/01/2018 13:44, Neil Horman: > > > On Fri, Jan 12, 2018 at 11:49:57AM +0000, Ferruh Yigit wrote: > > > > On 1/11/2018 8:50 PM, Neil Horman wrote: > > > > > On Thu, Jan 11, 2018 at 08:06:43PM +0000, Ferruh Yigit wrote: > > > > >> On 12/13/2017 3:17 PM, Neil Horman wrote: > > > > >>> --- a/app/test-eventdev/Makefile > > > > >>> +++ b/app/test-eventdev/Makefile > > > > >>> @@ -32,6 +32,7 @@ include $(RTE_SDK)/mk/rte.vars.mk > > > > >>> > > > > >>> APP = dpdk-test-eventdev > > > > >>> > > > > >>> +CFLAGS += -DALLOW_EXPERIMENTAL_APIS > > > > >> > > > > >> Do we need this internally in DPDK? For application developers this is great, > > > > >> they will get warning unless explicitly stated that they are OK with it. > > > > >> > > > > > I'm not sure what you're asking here. As I mentioned in the initial post, I > > > > > think we should give blanket permission to any in-tree dpdk library to allow the > > > > > use of experimental API's, but that doesn't really imply that all developers > > > > > would wan't it disabled all the time. That is to say, I could envision a > > > > > library author who, early in development would want to get a warning issued if > > > > > they used an unstable API, and, only once they were happy with their design and > > > > > choice of API usage, turn the warning off. > > > > > > > > I got your point, but I think whoever using an experimental API in another > > > > component in DPDK is almost always the author of the that experimental API, so > > > > not sure this check is ever really needed within dpdk. > > > > > > > I would have thought so too, but it doesn't really bear up. The example I used > > > to convince myself of a more granular approach was commit > > > 9c38b704d280ac128003238d7d80bf07fa556a7d where the rte_service API was > > > introduced as experimental by Nikhil Rao, and then later used in the eal library > > > as part of commit a894d4815f79b0d76527d9c42b23327de1501711 by Harry van Haaren. > > > Its no big deal because, as we agree, internal usage should be considered safe, > > > but it seemed clear that differing authors were using each others code > > > (potentially oblivious to the experimental nature of those APIs) > > > > > > > But OK, I guess it won't hurt to have more granular approach. > > > > > > > > > > > > > >> Do we have any option than allowing them in DPDK library? And when experimental > > > > >> API modified the users in the DPDK library internally guaranteed to be updated. > > > > >> Why not globally allow this for all DPDK internally? > > > > >> > > > > > For the reason I gave above. We certainly could enable this in a more top-level > > > > > makefile so that for in-library systems it was opt-in rather than opt-out, but I > > > > > chose a more granular approach because I could envision newer libraries wanting > > > > > it on. I also felt, generally speaking, that where warning flags were > > > > > concerned, it generally desireous to have them on by default, and make people > > > > > explicitly choose to turn them off. > > > > I think DPDK developpers look at the EXPERIMENTAL warning in the doxygen > > above the prototype. > I'm not sure I agree with that, but regardless, my initial reasoning for writing > this tag was to call attention to experimental API's during review, rather than > their use during development, so I didn't gripe about ABI changes on expemted > code. Additionally, weather they look at the docs or not, they can > pre-emptively turn off the warning if they choose. > > > And when API will be switched to stable, we probably won't remove the flag > > in the makefile to disable allowing experimental. > Well, that remains to be seen I suppose. > > > So at the end, we could just allow experimental API for all internal libs. > Thats a rather bootstrapping argument. You are effecitvely saying that no one > developing will ever want to be warned of using experimental APIs in DPDK, so > lets just turn it off everyone. I would really rather let individual developers > make that call at the time they author something new. I don't see the benefit, but I am OK to keep it like this.