GNOME Bugzilla – Bug 685338
JHBuild: New plugin for building and running programs inside the jhbuild environment.
Last modified: 2012-11-06 20:55:09 UTC
The following series of patches adds the ability to build and run programs inside a JHBuild environment. For it to work it currently requires the patch from https://bugzilla.gnome.org/show_bug.cgi?id=685337 to be applied to the jhbuild installation. Hopefully I'll soon be able to get the jhbuild patch merged so that this plugin easily can be used by gnome developers.
Created attachment 225630 [details] [review] IAnjutaEnvironment: add new get_environment_variables() function. To be used to get the base environment variables to use in AnjutaEnvironmentEditor.
Created attachment 225631 [details] [review] EnvironmentEditor: Add support for showing arbitrary envvars. Add new function anjuta_environment_editor_set_base_variables() to set the base variables to be shown instead of the default environment variables of the process.
Created attachment 225632 [details] [review] build-basics-autotools: Use envvars from IAnjutaEnvironment plugin if available.
Created attachment 225633 [details] [review] run-program: use AnjutaEnvironmentEditor instead of implementing our own version.
Created attachment 225634 [details] [review] run-program: Use envvars from IAnjutaEnvironment when running programs.
Created attachment 225635 [details] [review] jhbuild: new plugin to build/run program inside the jhbuild environment.
Review of attachment 225633 [details] [review]: Thank you for your patch, it's much better like this. I think I have written the version in the run plugin first and I have created the AnjutaEnvironmentEditor later. I suppose I have forgotten to change the run plugin to use it. I have already committed it.
Then, I think it's a very good idea to have a jhbuild plugin. I have just one question about the way, it's done. When I have done the IAnjutaEnvironment interface, I have in mind that a plugin has to implement the override method to change the environment, so no new function is needed. The run and the build plugin should call the override method to eventually get a modified environment before running any command. By example a jhbuild plugin could implement the override method to replace the build command "build_command" by "jhbuild run build_command". The environment editor wasn't supposed to be able to modified the environment. I imagine that jhbuild still keep the environment of its parent. So if you modify a environment variable of its parent, you should get it in jhbuild environment, unless jhbuild change this variable. But in this case it probably means that jhbuild could not work if the variable is modified. I'm agree that your ianjuta_environment_get_environment_variables is similar to the already existing ianjuta_environment_get_real_directory method which doesn't fit with the view above. But I have an issue with chroot environment. Ideally, I should have filtered the output of all commands run inside the environment to replace file paths inside the environment with file paths valid outside. It's quite difficult to do, especially because you don't know what is a file path and what is a simple text. That's why I have moved this part in the caller which, if it parses the command output, knows where are the file paths. I'm not sure that the existing definition of IAnjutaEnvironment (so using the ianjuta_environment_override method) is the best one. But I think it doesn't need a change in jhbuild. So why have you added another function? Is there some limitations? or perhaps the documentation was not clear enough? If yes, I hope these explanations are better.
So the reason I added the new ianjuta_environment_get_environment_variables method is because I kind of think it makes sense to show the user what environment variables really will be used. Using ianjuta_environment_override instead would "magically" alter the environment variables without the user knowing what the end result will be. Perhaps the user doesn't need to know this and in that case we could instead just run the commands with "jhbuild run" as Colin said in the jhbuild bug. Using "jhbuild run" we wouldn't need any changes to jhbuild for getting the environment variables but we would still need a way to get the "prefix" and "libdir" directory from jhbuild.
The IAnjutaEnvironment interface allows you to edit the altered environment variables if you want. It's just not implemented in this way currently. I'm still not sure if it's better to do it it or not but I see some advantages indeed. Do you think it's better for the user to edit these variables even with the possibility to break something? To mitigate this issue, we could perhaps think of a way to make some variable read only. The other issue of having a get_environment method instead of override, is that using the override method allows you to know the executed command. I have done this because I imagine that it's possible to set different environment variables depending on the command. By example, we can imagine that we build a program on Linux and execute it in another environment using a simulator by example needing a different environment. Then, I'm not sure that the current interface is really good for such thing. I think it can be a bit difficult to deduce from the command if it's a debug commands, a build commands or something else. Do you have some ideas to manage this? One last issue, that we can have, is what's happen if they are several environment plugin?
It seems that we don't need a change in jhbuild to implement this but I suppose you need to change your current patches. Do you have enough time to work on this?
Created attachment 226776 [details] [review] jhbuild: new plugin to build/run program inside the jhbuild environment. https://bugzilla.gnome.org/show_bug.cgi?id=685338 This is a simple plugin implementing IAnjutaEnvironent which runs all commands using "jhbuild run". It also appends "--prefix" and "--libdir" parameters to invocations of configure and ./autogen.sh.
I've attached a new simpler version that simply runs all commands using "jhbuild run" and adds "--prefix" and "--libdir" configure and ./autogen.sh. It also doesn't need any changes to JHBuild.
Review of attachment 226776 [details] [review]: Thanks for your patch. It's good but I get a crash here because the function ianjuta_environment_get_real_directory is not implemented. Do you see this on your system? In your case, I think just returning dir will be enough.
Created attachment 227029 [details] [review] jhbuild: new plugin to build/run program inside the jhbuild environment. https://bugzilla.gnome.org/show_bug.cgi?id=685338 This is a simple plugin implementing IAnjutaEnvironent which runs all commands using "jhbuild run". It also appends "--prefix" and "--libdir" parameters to invocations of configure and ./autogen.sh.
I have not experienced the crash you explained but I guess it's expected of implementers of IAnjutaEnvironment to implement all functions. I've attached a new version of the patch that does this.
Review of attachment 227029 [details] [review]: Thanks for your patch. I have just committed it without any changes.
Do you think it is useful to provide some GUI to configure by example jhbuild options? I think it can be seen as plugin preferences. Do you think we can improve the IAnjutaEnvironment interface?
I don't really think there's a need to have a GUI to configure jhbuild. Perhaps it would be useful to set the path to the jhbuild executable though so that you don't need to have it in your PATH.