> On Oct 11, 2016, at 8:25 PM, Matthew Ahrens <[email protected]> wrote:
> 
> 
> 
> On Fri, Sep 30, 2016 at 6:21 PM, Igor Kozhukhov <[email protected] 
> <mailto:[email protected]>> wrote:
> 
>> On Oct 1, 2016, at 2:00 AM, Matthew Ahrens <[email protected] 
>> <mailto:[email protected]>> wrote:
>> 
>> On Fri, Sep 30, 2016 at 12:54 PM, Igor Kozhukhov <[email protected] 
>> <mailto:[email protected]>> wrote:
>> rechecked now full list of changes - we are using sources in all places: 
>> cmd/lib/uts
>> 
>> i’d like propose move shared sources to src/common/lua
>> 
>> The lua source is used only for ZFS, so how is it "shared"?  Do you have 
>> plans for other kernel (or user) subsystems to use lua?
> 
> ZFS code is shared between cmd/lib/uts
> based on changes in Makefile in different components i can see references to 
> LUA sourcs.
> 
> for example: in LIBZPOOL:
> +OBJECTS=$(LUA_OBJS) $(ZFS_COMMON_OBJS) $(ZFS_SHARED_OBJS) $(KERNEL_OBJS)
> +LUA_SRCS=            $(LUA_OBJS:%.o=../../../uts/common/fs/zfs/lua/%.c)
> 
> to me : it is mistake. becasue for design of build system shared sources 
> should be located at $(SRC)/common - or i have mistake?
> 
> By this logic, all zfs kernel source code (usr/src/uts/common/fs/zfs/*.c) 
> should go in common because it is compiled into libzpool.  If you want to 
> make that argument, let's start another thread for it.  We have added the LUA 
> code in the proper place following existing precedent.
>  
> 
> in CMD components are the same references to LUA sources.
> 
> Where?  I'm not seeing any more references to LUA_OBJS in Makefiles except 
> for libzpool and the kernel.
> 
> you try to use the same mistake with ZFS shared sources files - they should 
> be moved to shared place instead of use of files from UTS place: by build 
> procedure for cmd/zfs cmd/zpool, lib/libzfs* i can see it and it is really 
> hard to understand how is ZFS code shared.
> 
> The LUA code shouldn't be built into cmd/zfs, cmd/zpool/, or lib/libzfs*.  If 
> it is then we will fix that, but I don't see that it is.

first place in example:
usr/src/cmd/mdb/intel/amd64/zfs/Makefile
+CPPFLAGS += -I../../../../../uts/common/fs/zfs/lua

it is cmd/mdb and it contain reference to uts for lua sources

next:
usr/src/cmd/zhack/Makefile.com <http://makefile.com/>
+INCS +=        -I../../../uts/common/fs/zfs/lua

it cmd/zhack - cintain reference to lua sources

usr/src/cmd/zinject/Makefile.com <http://makefile.com/>
+INCS +=        -I../../../uts/common/fs/zfs/lua

again

i don’t want copy/past all references to lua sources in cmd/* apps :)
just for your info.

as i said before - better location for shared sources in shared place - 
src/common/lua - it is my opinion based on current build system.
it will be easy update it if needed and reuse by others components in builds.

-Igor

> 
> --matt
>  
> and you try to build the same source with different build flags: i know it, 
> because i did more gcc warnings cleanups.
> much more better to use $(SRC)/common place where you can find shared sources 
> between components in different places.
> 
>> 
>> for private library add -sys and use it for cmd components.
>> 
>> I don't understand what you're suggesting here.  We aren't creating a lua 
>> command or library, it's completely baked in to the zfs kernel module (and 
>> for testing only, libzpool).
> 
> it was proposal for : if we have shared code between components - move it to 
> separate lua-sys library and use it.
> becasue it can be usable later for others components.
> if you have no shared code/fuctions - no need create library. 
> 
>> --matt
>>  
>> 
>> i did similar for acpica, but new structure was not approved.
>> 
>> -Igor
>> 
>>> i think, based on info that current modified lua is part of kernel modules 
>>> builds, will be better put sources to:
>>> uts/common/lang/lua/* - where we can save original sources structure for 
>>> next updates.
>>> 
>>> -Igor
>>> 
>>> > On Sep 30, 2016, at 10:16 PM, Chris Williamson <[email protected] 
>>> > <mailto:[email protected]>> wrote:
>>> > 
>>> > It was necessary to slightly modify the base lua 5.2.4 interpreter for a 
>>> > couple reasons:
>>> > 
>>> > the need to disable a number of printing, file io, and pcall functions
>>> > error handling changes to allow channel programs to return errors rather 
>>> > than panicking the kernel
>>> > limited kernel stack space
>>> > math compatibility functions since we've changed the number 
>>> > representation from long double to int64_t
>>> > a handful of inconsistencies in expected standard library signatures
>>> > From looking at the configuration options for the packaged Lua 
>>> > interpreter, unfortunately I don't think we'd be able to just use 
>>> > binaries from a userland package. As such we've added the full source.
>>> > 
>>> > I'm open to suggestions if there's a better home for the interpreter code 
>>> > than uts/common/fs/zfs/lua, though.
>>> > 
>>> > For reference, here's the diff between the stock Lua 5.2.4 interpreter 
>>> > and the modified one we've included:
>>> > https://gist.github.com/cwill/9b71422008c8c08ff091faefdcc0bc42 
>>> > <https://gist.github.com/cwill/9b71422008c8c08ff091faefdcc0bc42> 
>>> > <https://gist.github.com/cwill/9b71422008c8c08ff091faefdcc0bc42 
>>> > <https://gist.github.com/cwill/9b71422008c8c08ff091faefdcc0bc42>>
>>> > —
>>> > You are receiving this because you commented.
>>> > Reply to this email directly, view it on GitHub 
>>> > <https://github.com/openzfs/openzfs/pull/198#issuecomment-250829574 
>>> > <https://github.com/openzfs/openzfs/pull/198#issuecomment-250829574>>, or 
>>> > mute the thread 
>>> > <https://github.com/notifications/unsubscribe-auth/AA5Gk15JDTKJVautHBZus9cVgrrS-INSks5qvWAfgaJpZM4KISTa
>>> >  
>>> > <https://github.com/notifications/unsubscribe-auth/AA5Gk15JDTKJVautHBZus9cVgrrS-INSks5qvWAfgaJpZM4KISTa>>.
>>> > 
>>> 
>>> —
>>> You are receiving this because you are subscribed to this thread.
>>> Reply to this email directly, view it on GitHub 
>>> <https://github.com/openzfs/openzfs/pull/198#issuecomment-250835912>, or 
>>> mute the thread 
>>> <https://github.com/notifications/unsubscribe-auth/AOi0K2ml4gYLpOJ70A764h3tumQTQJpSks5qvWd_gaJpZM4KISTa>.
>>> 
>>> 
>>> 
>> 
>> 
> 
> 
> openzfs-developer | Archives 
> <https://www.listbox.com/member/archive/274414/=now>  
> <https://www.listbox.com/member/archive/rss/274414/28059325-76ffcbac> | 
> Modify <https://www.listbox.com/member/?&;> Your Subscription         
> <http://www.listbox.com/>



-------------------------------------------
openzfs-developer
Archives: https://www.listbox.com/member/archive/274414/=now
RSS Feed: https://www.listbox.com/member/archive/rss/274414/28015062-cce53afa
Modify Your Subscription: 
https://www.listbox.com/member/?member_id=28015062&id_secret=28015062-f966d51c
Powered by Listbox: http://www.listbox.com

Reply via email to