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

Basic package manager implementation #448

Open
wants to merge 4 commits into
base: develop
from

Conversation

@praj-foss
Copy link
Member

commented Jul 8, 2019

Summary

This contains a very basic package manager that provides installation, removal and syncing support for game packages. Currently, it only works with our old Jenkins but support for other types of providers is coming soon (not in this PR though). It supports all four types of game builds produced by Jenkins:

Fixes #447 on completion.

Testing

Coming soon

Roadmap

  • Support for unstable and stable builds of both Omega and normal packages
  • Fetching list of game versions available online
  • Caching and loading the version list
  • Downloading and installing game packages
  • Removing game packages
  • Integrating with original launcher code

praj-foss added some commits Jul 7, 2019

Support for getting game version list from Jenkins
- Adds methods to get game version list from Jenkins
- Defines enum for all four game types
- Adds docs and todos

@praj-foss praj-foss added the GSoC 2019 label Jul 8, 2019

@praj-foss praj-foss added this to the v4.0.0 - GSOC 2019 milestone Jul 8, 2019

@praj-foss praj-foss self-assigned this Jul 8, 2019

@praj-foss praj-foss added this to In progress in GSOC 2019: Launcher via automation Jul 8, 2019

@GooeyHub

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

Uh oh, something went wrong with the build. Need to check on that

@Cervator
Copy link
Member

left a comment

Had an initial look, knowing it isn't ready yet. Hopefully some of my comments are helpful instead of all already on the todo list :-)

OMEGA_STABLE("DistroOmegaRelease"),
OMEGA_UNSTABLE("DistroOmega");

// TODO: Remove this enum when 3rd-party package types are supported

This comment has been minimized.

Copy link
@Cervator

Cervator Jul 9, 2019

Member

Is the intent for the whole enum to go away in favor of JSON-based config files of some sort? I might suggest trying to work that out with even current state only supporting Jenkins.

Something like a module.txt but for packages, with description, url, provider type, etc. Might be something like that one piece to follow, even if there's no implementation (a design/convention from a different package manager that could be followed?)


private static final Logger logger = LoggerFactory.getLogger(JenkinsStorage.class);

private static final String JENKINS_JOB_URL = "http://jenkins.terasology.org/job/";

This comment has been minimized.

Copy link
@Cervator

Cervator Jul 9, 2019

Member

Much like the game package definition I would load these things from config files as well, not code (might already be intended for a future version?)

private final Path versionsCache;
private final Map<GamePackageType, List<Integer>> cachedVersions;

LocalStorage(Path localDir) {

This comment has been minimized.

Copy link
@Cervator

Cervator Jul 9, 2019

Member

No access modifier? Intentional or nah? :-)

This comment has been minimized.

Copy link
@praj-foss

praj-foss Jul 10, 2019

Author Member

IDE suggestions 😅

}
}

void updateCache(GamePackageType pkgType, List<Integer> versions) {

This comment has been minimized.

Copy link
@Cervator

Cervator Jul 9, 2019

Member

More access modifier. The IDE might sometime offer premature advice about what access level is needed, but it is rare to use anything beyond public or private (let alone package access level on purpose)

import java.nio.file.Path;

/**
* Handles installation, removal and update of game packages.

This comment has been minimized.

Copy link
@Cervator

Cervator Jul 9, 2019

Member

If this is the central-most class for the concept you might want to beef up the javadoc a fair amount 👍

This comment has been minimized.

Copy link
@praj-foss

praj-foss Jul 10, 2019

Author Member

It'll be fully documented by the end 👍

private final Storage onlineStorage;
private final LocalStorage localStorage;

public PackageManager(Path localDir) {

This comment has been minimized.

Copy link
@Cervator

Cervator Jul 9, 2019

Member

A point of confusion for me: Are you using LocalStorage as another option for where the launcher can get packages from (so essentially a "local provide") or as a target for packages to be stored locally? Where's localDir intended to come from?

This comment has been minimized.

Copy link
@praj-foss

praj-foss Jul 10, 2019

Author Member

For now, it is designed only to receive packages (so not a 'Provider') and to install/remove them from the filesystem. 🙂 The localDir path (that points to local data dir) already gets loaded by the LauncherInitTask, and the package manager will just borrow it from there

localStorage = new LocalStorage(localDir);
}

public void sync() {

This comment has been minimized.

Copy link
@Cervator

Cervator Jul 9, 2019

Member

Javadoc again is super useful all over especially in placeholder code :-)

/**
* Model of a game package.
*/
public class GamePackage {

This comment has been minimized.

Copy link
@Cervator

Cervator Jul 9, 2019

Member

Class or interface? I guess easy to change later.

This comment has been minimized.

Copy link
@praj-foss

praj-foss Jul 10, 2019

Author Member

Well, it has to end up in some POJO form. Will be decided soon 👍

@GooeyHub

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

Uh oh, something went wrong with the build. Need to check on that

Modifies package manager API
- Adds null checking
- Proper access modifiers
- Other structural modifications
@GooeyHub

This comment has been minimized.

Copy link
Member

commented Jul 21, 2019

Hooray Jenkins reported success with all tests good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.