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

Distribution is Java 1.8 #156

Closed
snodnipper opened this issue Oct 11, 2015 · 18 comments
Closed

Distribution is Java 1.8 #156

snodnipper opened this issue Oct 11, 2015 · 18 comments

Comments

@snodnipper
Copy link

I note that the description of the current released software states that:

"The package compiles on Java 1.2 thru Java 1.4"

That software appears, however, to be compiled with 1.8, which means the dependency cannot be used with legacy software (inc. Android, which only officially supports 1.7).

It would be great to target 1.6/1.7 but if 1.8 was chosen for specific reasons then perhaps the documentation could be updated to reflect that?

@stleary
Copy link
Owner

stleary commented Oct 11, 2015

@BGehrels Can you please update the POM file in the Maven repository to state that the package compiles with Java 1.8?

@stleary
Copy link
Owner

stleary commented Oct 11, 2015

@snodnipper Can you please clarify in what ways the lib is incompatible for building with Android?

Can you compile and use the latest source code with the Java 1.7 JDK?

@BGehrels
Copy link
Contributor

oh, yes, the sentence @snodnipper quoted was copied from the original README.md a long time ago. Since it is redundant to

        <plugin>
            <groupId>org.apache.maven.plugins</groupId>
            <artifactId>maven-compiler-plugin</artifactId>
            <version>2.3.2</version>
            <configuration>
                <source>1.8</source>
                <target>1.8</target>
            </configuration>
        </plugin>

I will probably remove it. This way i can't forget to update it with the next Java Major Release.

@johnjaylward
Copy link
Contributor

I can't think of anything in the code that would prevent compiling on anything lower than java 1.5

1.8 was chosen somewhat arbitrarily it seems, and no new features from the 1.6, 1.7 or 1.8 release have been specifically used.

If we wanted to we could probably target a lower JDK version for the compilation target to better support backwards compatibility.

for Example:

        <plugin>
            <groupId>org.apache.maven.plugins</groupId>
            <artifactId>maven-compiler-plugin</artifactId>
            <version>2.3.2</version>
            <configuration>
                <source>1.7</source>
                <target>1.7</target>
            </configuration>
        </plugin>

@johnjaylward
Copy link
Contributor

@stleary the compiled classes and jar released by @BGehrels would be targeting 1.8, but Android is run with class compatibility at 1.7. Jars compiled to 1.8 would not be able to run in an android environment I believe.

If @snodnipper compiled from source, there shouldn't be a problem.

@snodnipper
Copy link
Author

To ensure widest compatibility on Android with other legacy systems, targeting 1.6 would be great. I note that spring, for instance, is still backwards compatible with Java 6.

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-compiler-plugin</artifactId>
    <version>2.3.2</version>
    <configuration>
        <source>1.6</source>
        <target>1.6</target>
    </configuration>
</plugin>

@johnjaylward
Copy link
Contributor

ok, I found an instance where I used a java1.7 convention for exception handling in optEnum:

    public <E extends Enum<E>> E optEnum(Class<E> clazz, int index, E defaultValue) {
        try {
            Object val = this.opt(index);
            if (JSONObject.NULL.equals(val)) {
                return defaultValue;
            }
            if (clazz.isAssignableFrom(val.getClass())) {
                // we just checked it!
                @SuppressWarnings("unchecked")
                E myE = (E) val;
                return myE;
            }
            return Enum.valueOf(clazz, val.toString());
        } catch (IllegalArgumentException | NullPointerException e) {
            return defaultValue;
        }
    }

Looks like the lowest we can go is 1.7 without changing code.

@snodnipper
Copy link
Author

+1 - 1.7 seems like a reasonable compromise to me - that hits over 50% android devices (API 19 and above). Folk can always make a pull request or fork if 1.6 support is required.

@stleary
Copy link
Owner

stleary commented Oct 14, 2015

Will leave this open for a few more days til the POM file comment is fixed. Otherwise, it looks like at least the JSON-java source code will compile with 1.7, which maintains compatibility with the latest Android release.

@stleary
Copy link
Owner

stleary commented Oct 17, 2015

Hi @BGehrels, I see that the POM file at https://search.maven.org/#artifactdetails%7Corg.json%7Cjson%7C20150729%7Cjar has not been updated to remove the reference to Java 1.2-1.4. Is there any problem?

@BGehrels
Copy link
Contributor

Maven artifacts are immutable once publisher. What i can do is to publish a new release (2015100X) from the same sources using an altered pom Description.

@stleary
Copy link
Owner

stleary commented Oct 18, 2015

Thanks for the explanation. Since the Java build version is already specified, I think it should be OK to defer removing the line until the next release.

@jinahya
Copy link

jinahya commented Dec 6, 2015

I found there, within :20151123, are two multi-catch codes.
One at JSONArray#L597 and the other at JSONObject#L910.
After those two points are fixed, all sources can even be compiled for 1.6. See this pom.xml.

@johnjaylward
Copy link
Contributor

http://www.oracle.com/technetwork/java/eol-135779.html

Java 6 is eol as is java 7. Supporting java 7 for android support makes the most sense at this time. Going back and limiting features at 6 does not have a great use-case.

If you really need java 6 support, please make a case for it.

@jinahya
Copy link

jinahya commented Dec 9, 2015

I really don't think any EOL of a specific Java is relevant to source release compatibility(javac -source).
There are still so many devices and runtimes require lower class file versions.

  • Android Jelly Bean still takes more than 26% of market share. (I'm not an expert on Android platform.)
  • Plenty of old-timer STBs run on PBP(over CDC) which require 1.3 (This is what I tried to solve with json-retrotraslated by changing those two multi-catch blocks).

It will be more RI-tic if the source release compatibility is stick to 1.6 unless syntactic sugar, such as multi-catches, diamond-operators, are definitely required.

@johnjaylward
Copy link
Contributor

There are other benefits to using the newer versions of java, such as better compiler optimizations, resource handling (which this project doesn't use) etc.. At this time it's up to @stleary for a final decision, but I personally don't see a benefit of 1.6 over 1.7. However, I'm not doing work in the same areas as you.

If you have already made the change locally, then feel free to publish a pull request and it can be reviewed there.

@johnjaylward
Copy link
Contributor

Correction, this project does use resources that could benefit from the newer java7 resource management. Specifically the writer that converts the JSONObject and JSONArray classes into strings. At this time, I don't believe the resource management is used though.

@stleary
Copy link
Owner

stleary commented Dec 9, 2015

Unsubscribed viewers of the project won't even see this discussion because it is occurring on a closed issue with an unrelated title. We probably don't want to make 1.6 compatibility a project goal. But a small change that does not affect functionality or API and is backwards compatible with Java versions 1.6-1.7 is at least worth considering. Please submit your change as a pull request. If you note the compilers/execution environments you use for testing, that would support your proposal.

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

No branches or pull requests

5 participants