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 method for retrieving XDG config directory #2211

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import com.google.cloud.tools.jib.configuration.ImageConfiguration;
import com.google.cloud.tools.jib.docker.DockerClient;
import com.google.cloud.tools.jib.event.EventHandlers;
import com.google.cloud.tools.jib.filesystem.UserCacheHome;
import com.google.cloud.tools.jib.filesystem.XdgDirectories;
import com.google.common.base.Preconditions;
import java.io.IOException;
import java.nio.file.Files;
Expand All @@ -45,7 +45,7 @@ public class Containerizer {
* home]/google-cloud-tools-java/jib}.
*/
public static final Path DEFAULT_BASE_CACHE_DIRECTORY =
UserCacheHome.getCacheHome().resolve("google-cloud-tools-java").resolve("jib");
XdgDirectories.getCacheHome().resolve("google-cloud-tools-java").resolve("jib");

private static final String DEFAULT_TOOL_NAME = "jib-core";

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
/*
* Copyright 2018 Google LLC.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/

package com.google.cloud.tools.jib.filesystem;

import com.google.common.annotations.VisibleForTesting;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Locale;
import java.util.Map;
import java.util.Properties;
import java.util.logging.Logger;

/**
* Obtains OS-specific directories based on the XDG Base Directory Specification.
*
* <p>Specifically, from the specification:
*
* <ul>
* <li>These directories are defined by the environment variables {@code $XDG_CACHE_HOME} and
* {@code $XDG_CONFIG_HOME}.
* <li>If {@code $XDG_CACHE_HOME} / {@code $XDG_CONFIG_HOME} is either not set or empty, a
* platform-specific equivalent of {@code $HOME/.cache} / {@code $HOME/.config} should be
* used.
* </ul>
*
* @see <a
* href="https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html">https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html</a>
*/
public class XdgDirectories {

private static final Logger LOGGER = Logger.getLogger(XdgDirectories.class.getName());
private static final Path JIB_SUBDIRECTORY_LINUX =
Paths.get("google-cloud-tools-java").resolve("jib");
private static final Path JIB_SUBDIRECTORY_OTHER = Paths.get("Google").resolve("Jib");

public static Path getCacheHome() {
return getCacheHome(System.getProperties(), System.getenv());
}

public static Path getConfigHome() {
return getConfigHome(System.getProperties(), System.getenv());
}

/**
* Returns {@code $XDG_CACHE_HOME}, if available, or resolves the OS-specific user cache home
* based.
*
* <p>For Linux, this is {@code $HOME/.cache/}.
*
* <p>For Windows, this is {@code %LOCALAPPDATA%}.
*
* <p>For macOS, this is {@code $HOME/Library/Application Support/}.
*/
@VisibleForTesting
static Path getCacheHome(Properties properties, Map<String, String> environment) {
// Use environment variable $XDG_CACHE_HOME if set and not empty.
String xdgCacheHome = environment.get("XDG_CACHE_HOME");
if (xdgCacheHome != null && !xdgCacheHome.trim().isEmpty()) {
return Paths.get(xdgCacheHome);
}

String userHome = properties.getProperty("user.home");
Path xdgPath = Paths.get(userHome, ".cache");

String rawOsName = properties.getProperty("os.name");
String osName = rawOsName.toLowerCase(Locale.ENGLISH);

if (osName.contains("linux")) {
return xdgPath;

} else if (osName.contains("windows")) {
// Use %LOCALAPPDATA% for Windows.
String localAppDataEnv = environment.get("LOCALAPPDATA");
if (localAppDataEnv == null || localAppDataEnv.trim().isEmpty()) {
LOGGER.warning("LOCALAPPDATA environment is invalid or missing");
return xdgPath;
}
Path localAppData = Paths.get(localAppDataEnv);
if (!Files.exists(localAppData)) {
LOGGER.warning(localAppData + " does not exist");
return xdgPath;
}
return localAppData;

} else if (osName.contains("mac") || osName.contains("darwin")) {
// Use '~/Library/Application Support/' for macOS.
Path applicationSupport = Paths.get(userHome, "Library", "Application Support");
if (!Files.exists(applicationSupport)) {
LOGGER.warning(applicationSupport + " does not exist");
return xdgPath;
}
return applicationSupport;
}

throw new IllegalStateException("Unknown OS: " + rawOsName);
}

/**
* Returns config directory based on {@code $XDG_CONFIG_HOME}, if available, or resolves the
* OS-specific user config directory.
*
* <p>For Linux, this is {@code $HOME/.config/google-cloud-tools-java/jib/}.
*
* <p>For Windows, this is {@code %LOCALAPPDATA%\Google\Jib/}.
Copy link
Member

Choose a reason for hiding this comment

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

...\Google\Jib\ (instead of the trailing /)

*
* <p>For macOS, this is {@code $HOME/Library/Preferences/Google/Jib/}.
*/
@VisibleForTesting
static Path getConfigHome(Properties properties, Map<String, String> environment) {
Path windowsSubDirectory = JIB_SUBDIRECTORY_OTHER.resolve("Config");
Copy link
Member

Choose a reason for hiding this comment

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

Why not having windowsSubDirectory as a constant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's different for cache and config.

String rawOsName = properties.getProperty("os.name");
String osName = rawOsName.toLowerCase(Locale.ENGLISH);

// Use environment variable $XDG_CONFIG_HOME if set and not empty.
String xdgConfigHome = environment.get("XDG_CONFIG_HOME");
if (xdgConfigHome != null && !xdgConfigHome.trim().isEmpty()) {
if (osName.contains("linux")) {
return Paths.get(xdgConfigHome).resolve(JIB_SUBDIRECTORY_LINUX);
} else if (osName.contains("windows")) {
return Paths.get(xdgConfigHome).resolve(windowsSubDirectory);
}
return Paths.get(xdgConfigHome).resolve(JIB_SUBDIRECTORY_OTHER);
}

String userHome = properties.getProperty("user.home");
Path xdgPath = Paths.get(userHome, ".config");

if (osName.contains("linux")) {
Copy link
Member

Choose a reason for hiding this comment

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

We're now duplicating some of this platform-testing logic. Is it worth extracting into separate getOs(properties) method that returns the platform type? (I wish the JRE already provided this.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might be a good idea if we ever need it more. But I think with how little logic it actually is, only duplicating in 2 places isn't so bad.

return xdgPath.resolve(JIB_SUBDIRECTORY_LINUX);

} else if (osName.contains("windows")) {
// Use %LOCALAPPDATA% for Windows.
String localAppDataEnv = environment.get("LOCALAPPDATA");
if (localAppDataEnv == null || localAppDataEnv.trim().isEmpty()) {
LOGGER.warning("LOCALAPPDATA environment is invalid or missing");
return xdgPath.resolve(windowsSubDirectory);
}
Path localAppData = Paths.get(localAppDataEnv);
if (!Files.exists(localAppData)) {
LOGGER.warning(localAppData + " does not exist");
return xdgPath.resolve(windowsSubDirectory);
}
return localAppData.resolve(windowsSubDirectory);

} else if (osName.contains("mac") || osName.contains("darwin")) {
// Use '~/Library/Preferences/' for macOS.
Path applicationSupport = Paths.get(userHome, "Library", "Preferences");
if (!Files.exists(applicationSupport)) {
LOGGER.warning(applicationSupport + " does not exist");
return xdgPath.resolve(JIB_SUBDIRECTORY_OTHER);
}
return applicationSupport.resolve(JIB_SUBDIRECTORY_OTHER);
Copy link
Member

@chanseokoh chanseokoh Jan 7, 2020

Choose a reason for hiding this comment

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

Looks like this structure is being repeated.

if (linux) {
   return path.resolve(JIB_SUBDIRECTORY_LINUX);
} else if (windows) {
   return path.resolve(windowsSubDirectory);
} else /* if (mac) */ {
   return path.resolve(JIB_SUBDIRECTORY_OTHER);
}

Perhaps this logic can be a separate method, like getConfigHome(getBaseConfigHome(...)) (whatever suitable method names)? If this is indeed repeated, I think factoring this out will make the code more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I'll give it a try.

}

throw new IllegalStateException("Unknown OS: " + rawOsName);
}

private XdgDirectories() {}
}

This file was deleted.

Loading