Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move CraftOS out of bios.lua #347

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Conversation

ecktec
Copy link

@ecktec ecktec commented Jul 2, 2017

What does it do?

  • move the CraftOS parts from bios.lua into rom/boot.lua
  • at the end of bios.lua it adds checks for boot.lua and if it exists, it executes it, else it executes rom/boot.lua (CraftOS)
  • if some error occurs there, it prints it on standard term

How to use it?

  • replace CraftOS serverwide with a resourcepack replacing rom/boot.lua
  • replace CraftOS on a per computer base by creating a boot.lua file

How is that useful?

It is't; it's just a philosophical thing, but people always wanted to replace CraftOS instead of just hiding it.

@SquidDev
Copy link
Contributor

SquidDev commented Jul 2, 2017

Have you tested this actually works? From what I can tell, you're declaring bootfile in the if branches, which means it isn't in scope when you come to call it. Also fs.open takes a second argument and you never close the handle.

May be worth checking if boot isn't a directory and has no syntax errors before trying to do anything else.

@SquidDev
Copy link
Contributor

SquidDev commented Jul 2, 2017

Have you tested this actually works?

Sorry for being rather negative, but there do seem to be some other issues which would prevent this from working/being ideal.

  • The bootfile is loaded in an empty environment table.
  • The file handle is never closed, meaning you won't be able to edit the boot file when the computer is running.
  • boot should probably have a .lua file extension.
  • The bootfile should possibly be wrapped in a pcall, this is more up for debate :).

Again, sorry for all the complaints - it looks a good idea, we just need to make sure it all works OK.

@ecktec
Copy link
Author

ecktec commented Jul 2, 2017

I fully understand you, thank's for bringing up that many concerns ;-).
I'll try to fix as many things as possible.
I think it should just be boot without .lua , because startup also doesn't have a file extension.

@SquidDev
Copy link
Contributor

SquidDev commented Jul 2, 2017

CC now support startup with and without the file extensions. Dunno if it is worth supporting both here.

@ecktec
Copy link
Author

ecktec commented Jul 2, 2017

supporting boot and boot.lua would just require an additional elseif , I think it would be worth it.

@ecktec
Copy link
Author

ecktec commented Jul 2, 2017

And I don't want to use pcall, because I have no idea how to deal with errors on such a basic level.

@MineRobber9000
Copy link
Contributor

MineRobber9000 commented Jul 2, 2017

@ecktec regarding not knowing how to use pcall; see the original bios.lua as well as the pcall documentation.

Regarding this PR, I do not believe this is a good idea. Top-level coroutine overrides can be utilized to do this already.

@BombBloke
Copy link
Contributor

Personally, I would just rig this block here to check settings.get("bios.run") (or somesuch) for a table containing path strings, and if found, construct a bunch of os.run()ing functions out of the contents to pass to the parallel API. It would be nice, for example, to be able to easily load an alternate version of the rednet API which can work with a pre-shared key. Granted a TLCO can do that, but that's messy.

On the other hand, most users who write "OSes" for ComputerCraft don't know what an OS is, and produce things which could charitably be called "shells" (assuming they're even that complex). Their code typically runs on top of CraftOS and would break without it. Very few replace the shell API (or even realise that a proper CraftOS replacement would need to do that in order to remain compatible with standard ComputerCraft scripts), and I'd be surprised if more than a few users would be interested in re-writing anything you're moving into the "boot" file here.

@Lupus590
Copy link
Contributor

Lupus590 commented Jul 3, 2017

@BombBloke I was part way through a fork which did partly what you have described.

I was about to do a rewrite when I needed to work on a Uni resit.

Links:

@ecktec
Copy link
Author

ecktec commented Jul 3, 2017

@MineRobber9000 I meant I have no idea how to output it.

@BombBloke I think things like *nix kernels would get much better with that change, this is the purpose, if someone wants to replace the inner functions of CraftOS in a non hacky way, not replacing shell.

@JakobDev
Copy link
Contributor

JakobDev commented Jul 3, 2017

Please look at #272

Dan will only accept a PR, who makes bios.lua Changeable by Resourcepacks and not by normal users. So please remove the part who allow users to make their own bootfile.

btw. Please rename /rom/boot to /rom/boot.lua

@ecktec
Copy link
Author

ecktec commented Jul 3, 2017

@Wilma456 I think doesn't want anyone to be able to change it, because of the top part of bios.lua, which I kept there.
It would allow resourcepacks to replace rom/boot, which is the standard OS.

@Wojbie
Copy link
Contributor

Wojbie commented Jul 3, 2017

What Dan200 said is that he would prefer resource pack changing bios.lua.
It is unlikely that he will accept anything else.
If you really want this functionality that I suggest implementing that and then putting your own resource pack on forums with this bios.Kia.

@ecktec
Copy link
Author

ecktec commented Jul 3, 2017

People want to install OSes on single computers with vanilla computercraft.
Please stop speculating what @dan200 wants, just let him decide.

@Wojbie
Copy link
Contributor

Wojbie commented Jul 3, 2017

Quote from #272

dan200
commented about 1 month ago
It seems to me a simpler way to achieve these aims would just be to allow bios.lua to be overridden in resource packs like all the other lua files. I would accept a PR which did this

@ecktec
Copy link
Author

ecktec commented Jul 3, 2017

The problem is, that is doesn't achieve these aims, becuase people are lazy and don't want to have to install a resourcepack for that. It destroys the "just enter pastebin get this and this " culture of computercraft. "Download this zip file and place it in your resourcepack folder (in the .minecraft folder (where you get by pressing windows+s and entering %appdata% (or if you are using a different operating system than windows, like linux,
via something else) ) ), then activate it in the Resourcepack Settings."

@Lupus590
Copy link
Contributor

Lupus590 commented Jul 3, 2017

@Wojbie I'm going to just quote my original response to that comment that @dan200 posted:

One advantage over a resource pack bios that this will have is being able to have a different OS on different CC computers. Where as a new bios.lua makes changes server wide and can only be done by those with control of the server. quote source

@SquidDev
Copy link
Contributor

SquidDev commented Jul 3, 2017

Resource packs means the server owner has control over whether they want people to be able to run custom operating systems or not. Personally I think the resource pack option is the way to go, but then I've never really seen the point of custom operating systems.

@Lupus590
Copy link
Contributor

Lupus590 commented Jul 3, 2017

Could just make it a config option for if per computer override is possible. With Resource pack override being always possible.

@ecktec
Copy link
Author

ecktec commented Jul 3, 2017

you could override it by injecting something in rom/boot to not allow to have a boot file which would be very hacky

@MineRobber9000
Copy link
Contributor

@ecktec what about a per computer setting? (i.e; "set bios.use_bootfile true")

@ecktec
Copy link
Author

ecktec commented Jul 3, 2017

@MineRobber9000 The settings are defintely part of CraftOS. Also you can just not create a bootfile.

@ecktec
Copy link
Author

ecktec commented Jul 3, 2017

I just tested it ingame, it works as expected. (copied rom/boot and added a debug message)

@SquidDev
Copy link
Contributor

SquidDev commented Jul 3, 2017

Would it be worth printing a debug error message if boot has a syntax error or crashes whilst running? I'm less fussed about the latter, as the whole thing is meant to be "bare metal", but it would be useful.

@ecktec
Copy link
Author

ecktec commented Jul 4, 2017

I have no idea how to print an error message, because the print and write functions are both part of CraftOS. The Errors would come through load() (is there) and pcall() (not there, but easy to add), but I don't know how to output them. EDIT: ok, it's term.write()

@manu-03
Copy link

manu-03 commented Jul 4, 2017

I agree with this PR because it wouldn't prevent server owners from doing anything. They could simply change bios.lua in the resource pack if they didn't want to allow users to modify boot.lua. With these changes, players could make actual custom OSes that aren't sandboxed inside CraftOS. This would allow setting more files actually read-only and using features such as network booting with boot.lua being just like a boot manager.

@ecktec
Copy link
Author

ecktec commented Jul 4, 2017

@manu-03 You cannot change bios.lua with resourcepacks. This change would give server owners no more and no less control over what their users do. All your arguments are actually wrong, this change is just philosophical and has no more advantages than allowing less hacky code.

@CrazedProgrammer
Copy link
Contributor

Another issue, this could potentially brick a computer if the boot.lua is faulty and there is no access to the underlying file system (for example on a server).

@Lupus590
Copy link
Contributor

Lupus590 commented Jul 5, 2017

@CrazedProgrammer you can place computers in diskdrives, which will allow you to fix it

@CrazedProgrammer
Copy link
Contributor

Ahh, didn't think about that. This looks neat.

@ecktec
Copy link
Author

ecktec commented Jul 5, 2017

@CrazedProgrammer you can do the same pseudo-bricking with startup.lua already

@MineRobber9000
Copy link
Contributor

How about we use pcall and a failsafe? Something like:

local ok, err = pcall(function() os.run(bootfile,_G) end)
if not ok then term.write(err.."\n") sleep(1) os.run("rom/boot.lua",{}) end

Then, if the bootfile errors, it runs CraftOS, allowing the user to fix their error.

(Note: while sleep is a CraftOS defined function, how it works is trivial and can be implemented in the bios)

@ecktec
Copy link
Author

ecktec commented Jul 7, 2017

I also thought about that already, but I was not sure, if it should be there, but in case I implement it, i’ll also have to try them after each other, should I remove the ability to call it boot, currently it‘s like dan wants to convert the community to use .lua, I don‘t know.

@manu-03
Copy link

manu-03 commented Jul 10, 2017

@ektec no, they couldn't modify the BIOS, you have a point there. But it'd be as easy as adding an option so that the computer would just boot in legacy (capped) mode to the config file, which can be modified only by server owners.

@CrazedProgrammer
Copy link
Contributor

@ecktec Yes. There should only be a boot.lua.

@ecktec
Copy link
Author

ecktec commented Jul 12, 2017

@manu-03 This change will not open any new possibilities, I don't see any reason why someone would like to not enable it. You can do the same things in "legacy mode".

@manu-03
Copy link

manu-03 commented Jul 12, 2017

It's easy. It can lead to exploits if the server has limited OS functionality

@SquidDev
Copy link
Contributor

@manu-03 Can you give an example of such a potential exploit? None of the sandboxing-related code has been changed/deleted so this shouldn't be any more exploitable than the current system.

@manu-03
Copy link

manu-03 commented Jul 14, 2017

@SquidDev Just imagine a server owner wants to allow only certain computers (such as advanced ones or some specific IDs) to use HTTP. They can do it by three ways:

  1. Develop a server-side mod/plugin to cancel the requests which would need to interact directly with CC
  2. Change the startup.lua file so that all http-related functions check if the computer has these requirements
  3. Modify the HTTP API, directly inside the mod .jar

Even if the other two would still be avaliable, they are less accessible than the second one. Having an option to disable boot.lua (or force running /rom/startup.lua before it) would make the second one safe, while a simple boot.lua would just override the restriction.

@dmarcuse
Copy link
Contributor

@manu-03 That's what the autorun ROM folder is for. You can make a custom resource pack that has a program that overrides the http API.

@SquidDev
Copy link
Contributor

@apemanzilla That gets loaded from rom/startup though, which is only run from the shell API - and so this PR would allow people to prevent the http patch being overridden. However, if "bios.lua through resource packs" gets added (I'll have a look at putting together a PR over the weekend) then this isn't a problem.

@dmarcuse
Copy link
Contributor

Ah, I misread it. Thanks for clarifying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants