PULL] ADT installer script implementation, Liping Ke

PULL] ADT installer script implementation, Liping Ke

[Ke, Liping]

Hi, Saul

This pull request includes initial ADT(Yocto Application Development Tools) installer script implementation.

Pull URL: git://git.pokylinux.org/poky-contrib.git
Branch: lke/adt_installer_initial
Browse: http://git.pokylinux.org/cgit/cgit.cgi/poky-contrib/commit/?h=lke/adt_installer_initial&id=a06769b01d7b18894ad413daa8159f74c90f6c0f


Thanks,
criping



Add initial yocto adt installer scriptlke/adt_installer_initial
Below patches are for installing Yocto Application Development Tools.
opkg files include both 32 bit and 64 bit installation machine.
Installer entry script file is yocto_installer. A hidden .config files
are for holding parameters which are unlikely to be changed by end users

Signed-off-by: Liping Ke <liping.ke@intel.com>
Signed-off-by: Jessica Zhang<jessica.zhang@intel.com>
Diffstat (more/less context) (ignore whitespace changes)
-rw-r--r--	scripts/adt-installer/.config	35	
-rw-r--r--	scripts/adt-installer/opkg-sdk-i586.conf	17	
-rw-r--r--	scripts/adt-installer/opkg-sdk-x86_64.conf	17	
-rw-r--r--	scripts/adt-installer/util	70	
-rwxr-xr-x	scripts/adt-installer/yocto_installer	250	
-rw-r--r--	scripts/adt-installer/yocto_installer.conf	32	
-rwxr-xr-x	scripts/adt-installer/yocto_installer_internal	222	
7 files changed, 643 insertions, 0 deletions

Re: PULL] ADT installer script implementation, Liping Ke

[Richard Purdie]

Hi Liping, Jessica and team,

On Thu, 2010-12-09 at 17:28 +0800, Ke, Liping wrote:
> Pull URL: git://git.pokylinux.org/poky-contrib.git
> Branch: lke/adt_installer_initial
> Browse: http://git.pokylinux.org/cgit/cgit.cgi/poky-contrib/commit/?h=lke/adt_installer_initial&id=a06769b01d7b18894ad413daa8159f74c90f6c0f


I had a quick look over this and have a few comments. I'm in critical
review mode and don't have a lot of time so please don't take any of
this feedback the wrong way, I just want to give you honest first hand
first time user perception.

The first thing I read was in the patch was:

+# following are for testing in my machine. it will be moved when
+# going into code repository
+export http_proxy=""
+export HTTP_PROXY=""
+export HTTPS_PROXY=""
+export https_proxy=""
+export ftp_proxy=""
+export FTP_PROXY=""

which it evidently wasn't! ;-)

The rest of that file suggests it probably shouldn't be there actually?

I then decided to try it.

cd /
root@rex:/# /rphome/poky/scripts/adt-installer/yocto_installer
/rphome/poky/scripts/adt-installer/yocto_installer: line 207: .config: No such file or directory
[snip lots of similar errors]

So I'd suggest it looks at the location of the main script and then uses
this to find its config files.

It then found the presetup .config file and took those values as the
defaults. This was not obvious to me, it should perhaps really state
where its getting that config from.

I removed the .config file. That didn't help me so I removed
yocto_installer.conf. This didn't help me much either.

Now, reading the script I suddenly realise the user doesn't get prompted
to be able to change the values, you can only set these in the config
files.

I tried hitting Y for fun despite knowing it would crash and burn. Those
error messages were good so kudos there :)

I think it would make most sense for the user to be prompted for unset
values with "enter" selecting a sane set of defaults.

So in summary, I think the right pieces are there in terms of
configuration but the user interaction needs a lot more thought. I
didn't try an actual installation as I don't have a local setup to allow
it.

For the next review, I'd like an opkg repo to be available somewhere to
test against and for you to provide an installer tarball as an end user
would receive for people to evaluate.

Cheers,

Richard

Re: PULL] ADT installer script implementation, Liping Ke

[Zhang, Jessica]

[-- Attachment #1: Type: text/plain, Size: 3449 bytes --]

Richard Purdie wrote:
> Hi Liping, Jessica and team,
> 
> On Thu, 2010-12-09 at 17:28 +0800, Ke, Liping wrote:
>> Pull URL: git://git.pokylinux.org/poky-contrib.git
>> Branch: lke/adt_installer_initial
>> Browse:
>>
http://git.pokylinux.org/cgit/cgit.cgi/poky-contrib/commit/?h=lke/adt_instal
ler_initial&id=a06769b01d7b18894ad413daa8159f74c90f6c0f
> 
> 
> I had a quick look over this and have a few comments. I'm in critical
> review mode and don't have a lot of time so please don't take any of
> this feedback the wrong way, I just want to give you honest first hand
> first time user perception.

Really appreciated your time and feedbacks.

> 
> The first thing I read was in the patch was:
> 
> +# following are for testing in my machine. it will be moved when
> +# going into code repository
> +export http_proxy=""
> +export HTTP_PROXY=""
> +export HTTPS_PROXY=""
> +export https_proxy=""
> +export ftp_proxy=""
> +export FTP_PROXY=""
> 
> which it evidently wasn't! ;-)
> 
> The rest of that file suggests it probably shouldn't be there
> actually? 
> 

Sorry for this obvious mistake. Need to do a better job before sending our a
review request next time

> I then decided to try it.
> 
> cd /
> root@rex:/# /rphome/poky/scripts/adt-installer/yocto_installer
> /rphome/poky/scripts/adt-installer/yocto_installer: line 207:
> .config: No such file or directory [snip lots of similar errors]
> 
> So I'd suggest it looks at the location of the main script and then
> uses 
> this to find its config files.
> 
> It then found the presetup .config file and took those values as the
> defaults. This was not obvious to me, it should perhaps really state
> where its getting that config from.
> 
> I removed the .config file. That didn't help me so I removed
> yocto_installer.conf. This didn't help me much either.
> 

That definitely a bug regarding how to locate .config that we need to
address.  As to the different config files, the .config is on purpose non
user editable that's why we set it as hidden file by default.  And the
yocto_installer.conf is the customizable config user should change and we
have prompt for the user as regard to the configuration values to be used
and where they're from along the installation process...

> Now, reading the script I suddenly realise the user doesn't get
> prompted 
> to be able to change the values, you can only set these in the config
> files.
> 
> I tried hitting Y for fun despite knowing it would crash and burn.
> Those 
> error messages were good so kudos there :)
> 
> I think it would make most sense for the user to be prompted for unset
> values with "enter" selecting a sane set of defaults.
> 
Agree and will make improvement on that..

> So in summary, I think the right pieces are there in terms of
> configuration but the user interaction needs a lot more thought. I
> didn't try an actual installation as I don't have a local setup to
> allow 
> it.
> 
> For the next review, I'd like an opkg repo to be available somewhere

Lianhao has a testing opkg repo setup on his machine which mimic the release
package repo and we've been using it test things out

> to 
> test against and for you to provide an installer tarball as an end
> user 
> would receive for people to evaluate.

Yes, and will attach the tar ball with in the next review request
> 
> Cheers,
> 
> Richard

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 8455 bytes --]

Re: PULL] ADT installer script implementation, Liping Ke

[Ke, Liping]

> > The first thing I read was in the patch was:
> >
> > +# following are for testing in my machine. it will be moved when
> > +# going into code repository
> > +export http_proxy=""
> > +export HTTP_PROXY=""
> > +export HTTPS_PROXY=""
> > +export https_proxy=""
> > +export ftp_proxy=""
> > +export FTP_PROXY=""
> >
> > which it evidently wasn't! ;-)
> >
> > The rest of that file suggests it probably shouldn't be there
> > actually?
> >
> 
> Sorry for this obvious mistake. Need to do a better job before sending
> our a
> review request next time

Oh, I am sorry that I should not send the [pull] request, instead I should send a review request.
I meant to collect some review comments before finalize the code.

This is for testing. So I did not remove this part. When going into tree,
it will definitely be removed.

> 
> > I then decided to try it.
> >
> > cd /
> > root@rex:/# /rphome/poky/scripts/adt-installer/yocto_installer
> > /rphome/poky/scripts/adt-installer/yocto_installer: line 207:
> > .config: No such file or directory [snip lots of similar errors]
> >
> > So I'd suggest it looks at the location of the main script and then
> > uses
> > this to find its config files.
> >
> > It then found the presetup .config file and took those values as the
> > defaults. This was not obvious to me, it should perhaps really state
> > where its getting that config from.
> >
> > I removed the .config file. That didn't help me so I removed
> > yocto_installer.conf. This didn't help me much either.
> >
>

> That definitely a bug regarding how to locate .config that we need to
> address.  As to the different config files, the .config is on purpose
> non
> user editable that's why we set it as hidden file by default.  And the
> yocto_installer.conf is the customizable config user should change and
> we
> have prompt for the user as regard to the configuration values to be
> used
> and where they're from along the installation process...

Yes, this is a problem. After source .config (default location), if we
Failed to locate the file, I should prompt to exit instead of going forward.

> 
> > Now, reading the script I suddenly realise the user doesn't get
> > prompted
> > to be able to change the values, you can only set these in the config
> > files.
> >
> > I tried hitting Y for fun despite knowing it would crash and burn.
> > Those
> > error messages were good so kudos there :)
> >
> > I think it would make most sense for the user to be prompted for
> unset
> > values with "enter" selecting a sane set of defaults.
> >
> Agree and will make improvement on that..
OK, I will give default values for all those variables. If user needs to
change it, he can use a config file to override it. Thanks for the suggestion.

> 
> > So in summary, I think the right pieces are there in terms of
> > configuration but the user interaction needs a lot more thought. I
> > didn't try an actual installation as I don't have a local setup to
> > allow
> > it.
> >
> > For the next review, I'd like an opkg repo to be available somewhere
> 
> Lianhao has a testing opkg repo setup on his machine which mimic the
> release
> package repo and we've been using it test things out


Also I did not attach the opkg source tar ball, so the script can't be run
actually. I plan to dynamically fetch opkg source file when using bitbake
to generate adt-installer tarball. But for reviewing, next time, I will
include the opkg source file tarball in the installer tarball.


Thanks& Regards,
criping

> 
> > to
> > test against and for you to provide an installer tarball as an end
> > user
> > would receive for people to evaluate.
> 
> Yes, and will attach the tar ball with in the next review request
> >
> > Cheers,
> >
> > Richard

Re: PULL] ADT installer script implementation, Liping Ke

[Ke, Liping]

> That definitely a bug regarding how to locate .config that we need to
> address.  As to the different config files, the .config is on purpose
> non
> user editable that's why we set it as hidden file by default.  And the
> yocto_installer.conf is the customizable config user should change and
> we
> have prompt for the user as regard to the configuration values to be
> used
> and where they're from along the installation process...
> 

Hi, Jessica

For the configuration in .config file, I found it has problem too as a hidden file those days.
It might be missed in some file ops. I was thinking whether I should remove this file
and directly assign values in the common file (such as util script). Normally user will
not change it. If he's capable enough, of course he can change the opkg file name
as well as log file?

As for the rootfs default downloading value, I will remove it to the Yocto_installer.conf

Thanks & Regards,
criping

Re: PULL] ADT installer script implementation, Liping Ke

[Ke, Liping]

Hi, Richard & Jessica

Actually I am not sure user requirement about the configuration input UI interactive interface

One possible solution is 
1) For those can't be changed by users (supported archs, supported os, it is defined by us), I prefer no UI interaction, and hard code in script
2) For those could be changed, but rarely (log file location, image downloading temporary folder, opkg configuration filename), I will still let it in the configuration file, 
   If configuration file does not exist, I will give a default value. {Question here, do you need I prompt user each of those configurations, and in screen give another chance of changing instead of changing configuration file?)
3) For those need user input (mostly in yocto_installer.conf), if this file does not exist, directly prompt user errors and exit installation? Since without this file, we don't know user want to install what?
So still the question, if already having configuration file, we still need user to input something in screen besides Y or N?

Another question is that since we need to bitbake the installation tarball out. So we will fetch opkg source tarball by bitbake instead of providing a default one in the local folder. Is that right?

Thanks & Regards,
criping




> I think it would make most sense for the user to be prompted for unset
> values with "enter" selecting a sane set of defaults.
> 
> So in summary, I think the right pieces are there in terms of
> configuration but the user interaction needs a lot more thought. I
> didn't try an actual installation as I don't have a local setup to
> allow
> it.
>