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

Add a new "Change engine version" button to the ContainerOverviewPanel #1970

Merged
merged 2 commits into from
Jun 9, 2019

Conversation

madoar
Copy link
Collaborator

@madoar madoar commented Jun 5, 2019

This PR adds a new Change engine version button to the container overview panel.
Whenever this button is pressed the new changeVersion method of the used Engine implementation of the container is called. The signature of the new changeVersion method is as follows:

    /**
     * changes the engine version of the given container
     *
     * @param containerName the name of the container
     */
    void changeVersion(String containerName);

@ImperatorS79 do you need anything else to continue on #1832?^^

@ImperatorS79
Copy link
Contributor

No, I will test this ASAP !

@madoar
Copy link
Collaborator Author

madoar commented Jun 5, 2019

Be aware that you will get an exception if you have no changeVersion method in your engines implementation! I haven't tested it yet with such a method available... So if you still get an error feel free to tell me.

@ImperatorS79
Copy link
Contributor

I have "this.kill" is not a function :/

@madoar
Copy link
Collaborator Author

madoar commented Jun 5, 2019

Yes, the reason is, that the changeVersion is part of the engine implementation and not the engine object, which is accessible from the scripts.

The engine implementation can be found here:
https://github.com/PhoenicisOrg/scripts/blob/master/Engines/Wine/Engine/Implementation/script.js

The engine object can be found here:
https://github.com/PhoenicisOrg/scripts/blob/master/Engines/Wine/Engine/Object/script.js

Shortterm I think it is best to either check whether it is possible to go without this.kill() or to provide this functionality otherwise. Longterm I think we will change the engine implementation to fuse it with the corresponding engine object. On the Java side we then will most likely need forwarding methods to for the implementation/object methods. The longterm solution is currently blocked by #1857.

@ImperatorS79
Copy link
Contributor

ImperatorS79 commented Jun 5, 2019

OK, I tried this

changeVersion: function (containerName) {
    	var setupWizard = SetupWizard(InstallationType.ENGINES, "Change container's wine version" ,java.util.Optional.empty());
		
    	//this.kill();
        var workingContainerDirectory = this.getContainerDirectory(this.getWorkingContainer());//EDITED
    	var containerConfiguration = this._configFactory.open(workingContainerDirectory + "/phoenicis.cfg"); //EDITED
    	var architecture = containerConfiguration.readValue("wineArchitecture", "x86");
    	var operatingSystem = this._operatingSystemFetcher.fetchCurrentOperationSystem().getWinePackage();
    	
    	var wineJson = JSON.parse(this.getAvailableVersions());
    	var distributions = new Array();
    	var versions = new Array();
    	print("coucou");
    	wineJson.forEach(function (subPart) {
        	var parts = subPart.split("-");
        	if(parts[2] == architecture) {
            	distributions.push(parts[0]);
            	versions.push(new Array()); 
            	subPart.packages.forEach(function (winePackage) {
        		versions[distributions.size()].push(winePackage.version);
            	});
        	}
    	});
    	
    	print("mazette");
    	var selectedDistribution = wizard.menu(tr("Please select the distribution of wine."), distributions);
    	var selectedVersion = wizard.menu(tr("Please select the version of wine."), versions[distributions.indexOf(selectedDistribution)]);
    	subCategory = selectedDistribution + "-" + operatingSystem + "-" + architecture;
    	
    	this.install(subCategory, version);
	
    	var containerNameCleaned = containerName.replace(this._containerRegex, '');
    	var containerDirectory = this._winePrefixesDirectory + "/" + containerNameCleaned + "/";
	
    	var containerConfiguration = this._configFactory.open(containerDirectory + "/phoenicis.cfg");
	
    	containerConfiguration.writeValue("wineVersion", selectedVersion);
    	containerConfiguration.writeValue("wineDistribution", selectedDistribution);
    	containerConfiguration.writeValue("wineArchitecture", architecture);
    	//this.kill();
    },

and I got this:

[ERROR] org.phoenicis.multithreading.ControlledThreadPoolExecutorService (l.63) - engines.wine.engine.implementation:398 TypeError: Cannot read property "readValue" from undefined
        at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.ECMAErrors.error(ECMAErrors.java:57)
        at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.ECMAErrors.typeError(ECMAErrors.java:213)
        at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.ECMAErrors.typeError(ECMAErrors.java:185)
        at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.ECMAErrors.typeError(ECMAErrors.java:172)
        at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.Undefined.get(Undefined.java:161)
        at jdk.scripting.nashorn.scripts/jdk.nashorn.internal.scripts.Script$Recompilation$369$16067A$\^eval\_$cu1$restOf.changeVersion(engines.wine.engine.implementation:398)
        at jdk.nashorn.javaadapters.org_phoenicis_engines_Engine.changeVersion(Unknown Source)
        at org.phoenicis.javafx.components.container.control.ContainersFeaturePanel.lambda$changeEngineVersion$9(ContainersFeaturePanel.java:172)
        at org.phoenicis.engines.EnginesManager.lambda$getEngine$0(EnginesManager.java:71)
        at org.phoenicis.scripts.nashorn.NashornInteractiveSession.eval(NashornInteractiveSession.java:34)
        at org.phoenicis.scripts.interpreter.BackgroundScriptInterpreter.lambda$createInteractiveSession$1(BackgroundScriptInterpreter.java:43)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
        at java.base/java.lang.Thread.run(Thread.java:834)

Exception in thread "pool-3-thread-4" engines.wine.engine.implementation:398 TypeError: Cannot read property "readValue" from undefined
        at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.ECMAErrors.error(ECMAErrors.java:57)
        at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.ECMAErrors.typeError(ECMAErrors.java:213)
        at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.ECMAErrors.typeError(ECMAErrors.java:185)
        at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.ECMAErrors.typeError(ECMAErrors.java:172)
        at jdk.scripting.nashorn/jdk.nashorn.internal.runtime.Undefined.get(Undefined.java:161)
        at jdk.scripting.nashorn.scripts/jdk.nashorn.internal.scripts.Script$Recompilation$369$16067A$\^eval\_$cu1$restOf.changeVersion(engines.wine.engine.implementation:398)
        at jdk.nashorn.javaadapters.org_phoenicis_engines_Engine.changeVersion(Unknown Source)
        at org.phoenicis.javafx.components.container.control.ContainersFeaturePanel.lambda$changeEngineVersion$9(ContainersFeaturePanel.java:172)
        at org.phoenicis.engines.EnginesManager.lambda$getEngine$0(EnginesManager.java:71)
        at org.phoenicis.scripts.nashorn.NashornInteractiveSession.eval(NashornInteractiveSession.java:34)
        at org.phoenicis.scripts.interpreter.BackgroundScriptInterpreter.lambda$createInteractiveSession$1(BackgroundScriptInterpreter.java:43)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
        at java.base/java.lang.Thread.run(Thread.java:834)

@madoar
Copy link
Collaborator Author

madoar commented Jun 5, 2019

Where is containerConfiguration defined? I think the error is thrown in the line:

var architecture = containerConfiguration.readValue("wineArchitecture", "x86");

More information about the source for the error is shown in the Stacktrace line:

 at jdk.scripting.nashorn.scripts/jdk.nashorn.internal.scripts.Script$Recompilation$369$16067A$\^eval\_$cu1$restOf.changeVersion(engines.wine.engine.implementation:398)

I don't have your changes, therefore I can't check, but the error should be because of line 398 in the script engines.wine.engine.implementation.

@ImperatorS79
Copy link
Contributor

Ok it works now, but I will have to look at the wizard step ^^.

@ImperatorS79
Copy link
Contributor

Argh, I have subPart.split is not a function !

@madoar
Copy link
Collaborator Author

madoar commented Jun 5, 2019

subPart seems to me like a (json) object, not a string. You can access its content via (object) properties/fields.

@ImperatorS79
Copy link
Contributor

changeVersion: function (containerName) {
    	var wizard = SetupWizard(InstallationType.ENGINES, "Change " + containerName + " container wine version" ,java.util.Optional.empty());
    	this._wizard = wizard;
		
    	//this.kill();
    	var workingContainerDirectory = this.getContainerDirectory(this.getWorkingContainer());
    	var containerConfiguration = this._configFactory.open(workingContainerDirectory + "/phoenicis.cfg");
    	var architecture = containerConfiguration.readValue("wineArchitecture", "x86");
    	var operatingSystem = this._operatingSystemFetcher.fetchCurrentOperationSystem().getWinePackage();
    	
    	var wineJson = JSON.parse(this.getAvailableVersions());
    	var distributions = new Array();
    	var versions = new Array();
    	wineJson.forEach(function (subPart) {
        	var parts = subPart.name.split("-");
        	if(parts[2] == architecture) {
            	distributions.push(parts[0]);
            	versions.push(new Array()); 
            	subPart.packages.forEach(function (winePackage) {
        		versions[distributions.length-1].push(winePackage.version);
            	});
        	}
    	});
    	
    	var selectedDistribution = wizard.menu(tr("Please select the distribution of wine."), distributions);
    	var selectedVersion = wizard.menu(tr("Please select the version of wine."), versions[distributions.indexOf(selectedDistribution)]);
    	subCategory = selectedDistribution + "-" + operatingSystem + "-" + architecture;
    	
    	this.install(subCategory, version);
	
    	var containerNameCleaned = containerName.replace(this._containerRegex, '');
    	var containerDirectory = this._winePrefixesDirectory + "/" + containerNameCleaned + "/";
	
    	var containerConfiguration = this._configFactory.open(containerDirectory + "/phoenicis.cfg");
	
    	containerConfiguration.writeValue("wineVersion", selectedVersion);
    	containerConfiguration.writeValue("wineDistribution", selectedDistribution);
    	containerConfiguration.writeValue("wineArchitecture", architecture);
    },

the only problem is that distributions.indexOf(selectedDistribution) does not seems to work :/

@ImperatorS79
Copy link
Contributor

OK, i got it working, the only "problem" is that the version in the menu are not sorted and it is a bit difficult to choose one ^^.

@ImperatorS79
Copy link
Contributor

As stated in the scripts PR, everything works and the item are sorted.

@@ -54,9 +55,15 @@

/**
* The container engine controller
* TODO: remove and directly use `enginesManager` instead
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this something you want to fix before merging?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -154,6 +162,26 @@ public void deleteContainer(final ContainerDTO container) {
confirmMessage.showAndCallback();
}

public void changeEngineVersion(final ContainerDTO container) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Javadoc missing

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

- rename onChangeEngine to onChangeEngineVersion
@madoar madoar merged commit 9536490 into PhoenicisOrg:master Jun 9, 2019
@madoar madoar deleted the add-change-engine-version-button branch June 9, 2019 18:45
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.

3 participants