On Tue, 23 Jun 2015 17:57:38 +0100 Daniel Kolesa <[email protected]> said:
> On Tue, Jun 23, 2015 at 5:42 PM, Mike Blumenkrantz > <[email protected]> wrote: > > On Tue, Jun 23, 2015 at 12:24 PM, Daniel Kolesa <[email protected]> > > wrote: > > > >> On Tue, Jun 23, 2015 at 5:16 PM, Mike Blumenkrantz > >> <[email protected]> wrote: > >> > It would be great if, in the future, you could refrain from reverts like > >> > this. An ongoing mailing list discussion was taking place, and, > >> regardless > >> > of what you feel "should never be done", now the issue is once again > >> > unsolved. > >> > >> In my experience if a bad solution is left in the tree for too long, > >> it'll remain there forever, thus I choose to revert it now. This is > >> one of the most awful pieces of code I've seen in the EFL yet and I > >> couldn't sleep at night if it was left in, so at least this allows me > >> to have a peace of mind; the issue was never really "solved" in the > >> first place anyway - touching internal symbols within libraries is a > >> big no (I'm sure most of other developers here will agree with my > >> statement) and I would appreciate if you discussed things like this on > >> the ML before actually introducing them. > >> > >> Regards, > >> > >> D5 > >> > >> > > I think this is a very misleading reply. Ignoring the obvious (and quite > > unnecessary) use of hyperbole, there are plenty of ways to ensure that code > > doesn't remain in the tree forever. I'll list a few of them so that you can > > use it as a reference in the event that you have this urge to revert again > > in the future: > > > > * Participate in ongoing mailing list discussions > > * Open a ticket on the bug tracker > > * Engage in discussion on IRC > > It's funny it's you who's telling me this... hey - enough of the snarking. mike is right. there is a reason i replied to the mail and didn't just revert. i was expecting a "hmm ok - i see" at least, then a revert either by mike, or by me or someone else after that. reverting imho is ok IF the commit does seriously break something. people pull updates from git all the time and if a break is going to leave others in a seriously broken setup, then indeed revert rapidly to get back to working, and resolve the issue after that emergency fix. this is not such a case. > > Your claim that the issue was not solved is also inaccurate. This is > > exactly how the symbol is accessed in other parts of the tree, > > You sure? http://pastebin.com/UVUdVuS2 > > The symbol is only accessed within efreet, which is fine. I'm talking > about cross-library internal symbol accesses, which IS bad, and it's > not being done anywhere in the EFL. > > > and having > > this ability is one of the great benefits of the unified source tree that > > we have. Considering that Edje already depends on Efreet for function > > calls, the worst that could happen here is that calling edje_init() at some > > point in the future causes a crash--something which would be instantly > > noticeable since a general efl tree build would then fail. > > That doesn't matter. Using internal symbols across libraries like this > is bad by definition and should not be encouraged. > > > > > If you can provide me with a comprehensive list of "things like this" which > > bother you when they are committed, I can try to initiate discussion in the > > community for future cases. > > That's ridiculous... that this is an ugly hack should be common sense, > and if you're unsure about how to make it non-hacky, just initiate a > discussion and we can try to work it out some other way. in this case the root cause is not related to this symbol. i agree that digging into the internals of efreet from outside of it is bad/ugly and even if it was formalized as an api call, it is working around a core issue (dbus session requirements). > >> > On Tue, Jun 23, 2015 at 5:22 AM, Daniel Kolesa <[email protected]> > >> wrote: > >> > > >> >> q66 pushed a commit to branch master. > >> >> > >> >> > >> >> > >> http://git.enlightenment.org/core/efl.git/commit/?id=d9db8888ac51175e350853597b3b317e9774a00a > >> >> > >> >> commit d9db8888ac51175e350853597b3b317e9774a00a > >> >> Author: Daniel Kolesa <[email protected]> > >> >> Date: Tue Jun 23 10:22:36 2015 +0100 > >> >> > >> >> Revert "edje: unset efreet cache update flag to prevent dbus > >> >> connections" > >> >> > >> >> This reverts commit 1edb35fff3fe54ac7eea1ba2c26e509284b4e470. > >> >> > >> >> Accessing symbols from other libs like this should never be done. > >> The > >> >> consequences of this are not significant enough to do this; better > >> >> solution > >> >> can be found but definitely not like this. > >> >> --- > >> >> src/lib/edje/edje_main.c | 2 -- > >> >> 1 file changed, 2 deletions(-) > >> >> > >> >> diff --git a/src/lib/edje/edje_main.c b/src/lib/edje/edje_main.c > >> >> index a891ec5..03c46ea 100644 > >> >> --- a/src/lib/edje/edje_main.c > >> >> +++ b/src/lib/edje/edje_main.c > >> >> @@ -1,6 +1,5 @@ > >> >> #include "edje_private.h" > >> >> > >> >> -extern int efreet_cache_update; > >> >> static Edje_Version _version = { VMAJ, VMIN, VMIC, VREV }; > >> >> EAPI Edje_Version * edje_version = &_version; > >> >> > >> >> @@ -70,7 +69,6 @@ edje_init(void) > >> >> goto shutdown_eet; > >> >> } > >> >> > >> >> - efreet_cache_update = 0; > >> >> if (!efreet_init()) > >> >> { > >> >> ERR("Efreet init failed"); > >> >> > >> >> -- > >> >> > >> >> > >> >> > >> > > >> ------------------------------------------------------------------------------ > >> > Monitor 25 network devices or servers for free with OpManager! > >> > OpManager is web-based network management software that monitors > >> > network devices and physical & virtual servers, alerts via email & sms > >> > for fault. Monitor 25 devices for free with no restriction. Download now > >> > http://ad.doubleclick.net/ddm/clk/292181274;119417398;o > >> > _______________________________________________ > >> > enlightenment-devel mailing list > >> > [email protected] > >> > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > >> > >> > >> ------------------------------------------------------------------------------ > >> Monitor 25 network devices or servers for free with OpManager! > >> OpManager is web-based network management software that monitors > >> network devices and physical & virtual servers, alerts via email & sms > >> for fault. Monitor 25 devices for free with no restriction. Download now > >> http://ad.doubleclick.net/ddm/clk/292181274;119417398;o > >> _______________________________________________ > >> enlightenment-devel mailing list > >> [email protected] > >> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > >> > > ------------------------------------------------------------------------------ > > Monitor 25 network devices or servers for free with OpManager! > > OpManager is web-based network management software that monitors > > network devices and physical & virtual servers, alerts via email & sms > > for fault. Monitor 25 devices for free with no restriction. Download now > > http://ad.doubleclick.net/ddm/clk/292181274;119417398;o > > _______________________________________________ > > enlightenment-devel mailing list > > [email protected] > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > ------------------------------------------------------------------------------ > Monitor 25 network devices or servers for free with OpManager! > OpManager is web-based network management software that monitors > network devices and physical & virtual servers, alerts via email & sms > for fault. Monitor 25 devices for free with no restriction. Download now > http://ad.doubleclick.net/ddm/clk/292181274;119417398;o > _______________________________________________ > enlightenment-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- The Rasterman (Carsten Haitzler) [email protected] ------------------------------------------------------------------------------ Monitor 25 network devices or servers for free with OpManager! OpManager is web-based network management software that monitors network devices and physical & virtual servers, alerts via email & sms for fault. Monitor 25 devices for free with no restriction. Download now http://ad.doubleclick.net/ddm/clk/292181274;119417398;o _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
