[Toybox] [PATCH] roadmap update: Android switched to toybox ls today.

[enh]

On Sat, May 16, 2015 at 3:23 PM, Rob Landley <rob at landley.net> wrote:

> On 05/16/2015 03:43 PM, enh wrote:
> > actually, this is the one outstanding patch you _shouldn't_ apply --- i
> > had to revert the switch to ls late last night after it broke a
> > surprising amount of infrastructure.
>
> Yeah, I was kinda surprised. I applied your first "add selinux" patch
> verbatim, but it builds with a warning under glibc.


that's not "under glibc" so much as "without selinux". this seems to fix
that:

diff --git a/lib/portability.h b/lib/portability.h
index aa1ee48..4c4a6e7 100644
--- a/lib/portability.h
+++ b/lib/portability.h
@@ -251,7 +251,8 @@ pid_t xfork(void);
 #include <selinux/selinux.h>
 #else
 #define is_selinux_enabled() 0
-int getcon(void* con);
+int getcon(void *con);
+int lgetfilecon(const char *path, char **con);
 #endif

 #if CFG_TOYBOX_SMACK



> I have pending stuff
> to fix in it, I posted my notes from my first look at it...
>
> > (the toolbox ls didn't columnate,
>
> I think I fixed that one here:
>
>
> https://github.com/landley/toybox/commit/b18c7e8a59e61b8ec49e2d0f8cb2dda19b8f6a82
>

no, *toolbox* didn't. toybox ls was actually more compatible when -C didn't
work properly :-)


> > and "adb shell ..." creates a pty on
> > the remote side, so toybox ls sees that and defaults to -C rather than
> > -1. callers can't use -1 to say what they want, because toolbox ls
> > didn't support that, and even if we add it today they still need to deal
> > with a mix of old and new devices.
>
> In theory it should do the ANSI size probe to talk to your xterm. You
> could also have "adb shell" export COLUMNS=1 to force -1 mode?
>
> (I'm not sure what the desired behavior is here in "given x, toybox
> should do y" terms. Is "adb shell" being used to run scripts? Provide an
> interactive terminal? Dunno the use case.)


"adb shell" is interactive. "adb shell ..." isn't.


> > similarly, the pty creation is on the
> > adbd side, so although i'll fix that to be like ssh(1), that will only
> > help new devices.
>
> I'm confused: you get toybox when you upgrade to android m, what
> infrastructure is _not_ upgraded when you do that, causing the version
> skew?


test infrastructure. IDEs. that kind of thing.


>
> > i suspect we'll have to disable the isatty test in
> > ls_main for the next ten years.
>
> In theory isatty() can also be taught "if the terminal size is 0 by 0
> when you query it, it's not real".
>
> It's easy enough to make a kconfig switch so the behavior defaults to -1
> instead of -C even when you have a terminal. (Or the test can be if
> !CFG_TOYBOX_ANDROID...) I'd just like to understand th problem space a
> little better first?


oh, me too. i almost didn't write anything but thought that would just lead
to questions, so i wrote the parenthetical explanation of why i'd had to
revert in the hopes that would waste _less_ time. oops.

short version: toybox is doing nothing wrong, i'm going to see how much
infrastructure i can fix (if i can fix ddmlib, IDEs and the CTS stuff both
just use that), and i'll come back with a specific patch if there's
something i can't fix.


>
> > there's also code that parses the
> > toolbox ls -l format, which doesn't have the "nlink" column, so i'll
> > need to find and fix those regular expressions. but first i need to talk
> > to the ddmlib folks to see why they're parsing ls output at all, when
> > using the adb sync protocol queries would seem like a better idea...)
>
> We can add a flag to switch off that column, but given that posix is the
> one that said to have that column:
>
> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/ls.html
>
> That may not be the ideal solution...


indeed, and i suspect this is the easiest thing to fix. something like
(?:\s+\d+)? should suffice.


>
> > but there's plenty of other stuff like the hwclock fix and the find
> > -inum addition and the xxd for pending to keep you busy,
>
> I'm trying to catch up, but I get on a plane back to japan tomorrow
> afternoon.
>
> (Also, I'm supposed to port $DAYJOB's kernel to smp for sh2 (which is
> fun because sh2 hasn't got an an atomic test-and-set so we may need to
> change the vhdl again), and add dma to our drivers now the dma engine
> can actually generate a completion interrupt, and finish forward porting
> the the kernel to 4.0 (the ethernet driver uses obsolete infrastructure
> to fake hardware timestamps and it broke for the third time between 3.19
> and 4.0, I need to understand it so I can rip it out. Plus I spent half
> the week unsuccessfully trying to make the h8 port's updated elf2flt
> repo work with my last gplv2 release binutils 2.17, and I'm _still_
> learning how bits of linking work...)
>
> > plus -- as you
> > said -- there's smack for the flight :-) (rather than make things more
> > confusing and risk slowing things down even further, i'm waiting until
> > the smack changes are in before i worry about SELinux. what i've seen
> > looks plausible though so i'm not anticipating problems.)
>
> There's a first pass review of that already posted, working on some of
> that today...
>
> > btw, on the subject of testing --- i noticed that a command that SEGVs
> > gets a "PASS". i've been meaning to fix that and send a patch, but since
> > it's been two weeks since i noticed already, i should at least report
> it...
>
> Huh. It shouldn't. (Unless it was expected to produce no output? Output
> to stderr can get ignored, and the return value's only meaningful if you
> explicitly test for it ala "command && echo yes"...)
>
> Where did you notice this? (Context?)


in my new tests for xxd. at one point xxd was segfaulting but the test
would say PASS.

checking again, it looks like it's just this test:

testing "xxd file2" "xxd file2" "" "" ""

(file2 is an empty file, and the test is to check that we don't output an
address until we have some data.)

something like this removes the gotcha:

diff --git a/scripts/runtest.sh b/scripts/runtest.sh
index 8da1089..b9302b0 100644
--- a/scripts/runtest.sh
+++ b/scripts/runtest.sh
@@ -86,6 +86,11 @@ testing()
   echo -ne "$5" | eval "$2" > actual
   RETVAL=$?

+  if [ $RETVAL -gt 128 ] && [ $RETVAL -lt 255 ]
+  then
+    echo "exited with signal (or returned $RETVAL)" >> actual
+  fi
+
   cmp expected actual > /dev/null 2>&1
   if [ $? -ne 0 ]
   then



> Rob
>



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20150516/22dea3be/attachment-0001.htm>