JOSM r19101 doesn't start

[GerdP]

After an svn update to r19101 JOSM no longer works.
Seems that one of the changes in org.openstreetmap.josm.tools.Utils is the cause.
Below is the console log.

BTW: JOSM opens the BugReportSender dialog but the "Report Bug" button doesn't work in this case. It just
produces the last stack trace
Also a lot of unit tests fail.

c:\josm\core>java -Xmx4G --add-exports=java.base/sun.security.action=ALL-UNNAMED --add-exports=java.desktop/com.sun.imageio.plugins.jpeg=ALL-UNNAMED --add-exports=java.desktop/com.sun.imageio.spi=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED -jar dist\josm-custom.jar
2024-06-12 08:59:01.257 INFORMATION: Protokollierungsgrad ist bei INFORMATION (INFO, 800)
2024-06-12 08:59:01.710 SEVERE: Handled by bug report queue: java.lang.NullPointerException
java.lang.NullPointerException
        at java.base/java.util.ImmutableCollections$ListN.indexOf(ImmutableCollections.java:716)
        at java.base/java.util.ImmutableCollections$AbstractImmutableList.contains(ImmutableCollections.java:329)
        at org.openstreetmap.josm.spi.preferences.MapListSetting.consistencyTest(MapListSetting.java:41)
        at org.openstreetmap.josm.spi.preferences.MapListSetting.<init>(MapListSetting.java:24)
        at org.openstreetmap.josm.spi.preferences.MapListSetting.copy(MapListSetting.java:35)
        at org.openstreetmap.josm.spi.preferences.MapListSetting.copy(MapListSetting.java:16)
        at org.openstreetmap.josm.data.Preferences.getSetting(Preferences.java:755)
        at org.openstreetmap.josm.spi.preferences.AbstractPreferences.getListOfMaps(AbstractPreferences.java:135)
        at org.openstreetmap.josm.data.StructUtils.getListOfStructs(StructUtils.java:101)
        at org.openstreetmap.josm.tools.PlatformHookWindows.extendFontconfig(PlatformHookWindows.java:477)
        at org.openstreetmap.josm.tools.PlatformHookWindows.afterPrefStartupHook(PlatformHookWindows.java:152)
        at org.openstreetmap.josm.gui.MainApplication.mainJOSM(MainApplication.java:906)
        at org.openstreetmap.josm.gui.MainApplication$3.processArguments(MainApplication.java:285)
        at org.openstreetmap.josm.gui.MainApplication.main(MainApplication.java:788)

2024-06-12 08:59:02.404 SEVERE: java.lang.IllegalStateException: HTTP factory has not been set
java.lang.IllegalStateException: HTTP factory has not been set
        at org.openstreetmap.josm.tools.HttpClient.create(HttpClient.java:564)
        at org.openstreetmap.josm.tools.HttpClient.create(HttpClient.java:552)
        at org.openstreetmap.josm.io.CachedFile.checkLocal(CachedFile.java:491)
        at org.openstreetmap.josm.io.CachedFile.getFile(CachedFile.java:284)
        at org.openstreetmap.josm.io.CachedFile.getInputStream(CachedFile.java:227)
        at org.openstreetmap.josm.io.CachedFile.getByteContent(CachedFile.java:245)
        at org.openstreetmap.josm.gui.bugreport.JosmUpdatePanel.getTestedVersion(JosmUpdatePanel.java:70)
        at org.openstreetmap.josm.gui.bugreport.JosmUpdatePanel.readCurrentVersion(JosmUpdatePanel.java:53)
        at java.base/java.lang.Thread.run(Thread.java:833)

2024-06-12 08:59:05.832 SEVERE: Handled by bug report queue: java.lang.IllegalStateException: HTTP factory has not been set
java.lang.IllegalStateException: HTTP factory has not been set
        at org.openstreetmap.josm.tools.HttpClient.create(HttpClient.java:564)
        at org.openstreetmap.josm.tools.bugreport.BugReportSender.pasteDebugText(BugReportSender.java:118)
        at org.openstreetmap.josm.tools.bugreport.BugReportSender.run(BugReportSender.java:92)

[GerdP]

This patch reverts just the change in Utils.toUnmodifiableList(). It makes JOSM start again, but why?

[GerdP]

OK, seems List.of() returns a collection which doesn't allow null values (it will crash if a null value is found)
and the null check in MapListSetting.consistencyTest() will always throw an NPE on such a collection.

I also don't understand the old code. I don't think that Arrays.asList() returns an unmodifyable list.

[gaben]

It seems only a Windows issue, I can still run on my dev env on Linux.

Close #23726?

[GerdP]

Ah, sorry, did not see your ticket. I agree we can close that as a duplicate of this ticket.
The code which crashes on startup on Windows is not executed on linux, but the code in Utils is still incompatible.

[gaben]

Ticket #23726 has been marked as a duplicate of this ticket.

[taylor.smock]

FML. I hate OS specific issues. I'll apply the patch from gaben until I get around to figuring out what is sending null values to the method.

[gaben]

*patch is GerdP's work

[GerdP]

until I get around to figuring out what is sending null values to the method.

This actually doesn't happen. The search for null throws the NPE.

[taylor.smock]

Replying to gaben:

*patch is GerdP's work

Sorry, I just got up. :(

[taylor.smock]

In 19102/josm:

Revert Utils.toUnmodifiableList changes from r19101 (see #23727, patch by GerdP, modified with extra context)

[GerdP]

I really think the special case in the Windows version is not the problem here. It's the implementation of List.of() which returns a collection that
a) doesn't allow null values and
b) doesn't allow to search for null (no idea why that throws an NPE istead of returning a not-found result)

One simply doesn't expect this when calling Utils.toUnmodifiableList()

[taylor.smock]

I was able to reproduce on mac during editing.
Steps to reproduce:

1. Start JOSM
2. Download test area
3. Map something that you will use in utilsplugin2 Replace geometry
4. Replace geometry (I did a building and an address node)

As far as not allowing null, I had assumed that the original author had read the API docs for List.of which explicitly calls out the fact that null elements are not allowed.

I also don't know why the immutable collections are throwing an NPE when checking to see if they contain null (I mean, I see the code, but I don't know why they don't just return a "sane" value of false).

I think I also need to revert the changes for toUnmodifiableMap.

EDIT: NVM on the changes for toUnmodifiableMap; it looks like any issues with that were fixed long ago, since all I did there was remove Java 8/9 code differences.

[gaben]

JOSM still cannot start on Windows with r19101 :(

[taylor.smock]

Did you mean r19102, or r19101? r19102 should have fixed the problem.

[nixill]

r19101 is still listed as latest on the front page and at https://josm.openstreetmap.de/josm-latest.jar

[taylor.smock]

AFAIK, there isn't a "good" way to trigger a new latest build for general consumption.

Last I knew, it was on a timed job (I suspect cronjob) which runs daily at some time.

@stoecker knows a lot more about that (and maybe if there is a way to trigger an update for josm-latest).

It would be nice (tm) if we could do the josm-latest updates via a jenkins job (which can be manually triggered), but I suspect stoecker has his reasons for not doing that.

[gaben]

Replying to taylor.smock:

Did you mean r19102, or r19101? r19102 should have fixed the problem.

Ah sorry you are right, I didn't crosscheck the latest version. I thought enough time passed to trigger the build for r19102, therefore I tested r19101.

[taylor.smock]

There is always the snapshot builds available on GitHub (see ​https://github.com/JOSM/josm/releases/tag/19102 ).

[stoecker]

Replying to taylor.smock:

AFAIK, there isn't a "good" way to trigger a new latest build for general consumption.

Last I knew, it was on a timed job (I suspect cronjob) which runs daily at some time.

Jupp. A cron at 3:30 CET calling "make" :-).

@stoecker knows a lot more about that (and maybe if there is a way to trigger an update for josm-latest).

Only way is to ask me ;-) I triggered a build now.

It would be nice (tm) if we could do the josm-latest updates via a jenkins job (which can be manually triggered), but I suspect stoecker has his reasons for not doing that.

Jupp. While it is possible to do "each revision releases" I still think developers should have a chance to make errors. Thus once daily is a good compromise.

Also a build does a bit more than creating the JAR (always from scratch, no partial builds), so server load also is a (small) issue.

[gaben]

It's fine IMO, just make the latest build easier to trigger manually.

What will happen if you get hit by a bus? ​https://xkcd.com/2347/

[stoecker]

In an case of bus hitting (probably rather a motorbike accident in my case) contacting FOSSGIS server sponsor (i.e. Frederik Ramm) will result in emergency transfer of maintainership.

Manual latest triggers are usually only necessary when tickets appear in the dozens. This here is relatively inoffensive ;-)

[taylor.smock]

I'll go ahead and close this since r19102 is now available for josm-latest.

What will happen if [stoecker gets] hit by a bus? ​​https://xkcd.com/2347/

1. Assume stoecker is on an extended vacation.
2. After some time (a month or two), panic and try poking Don-vip (does he still have server access?).
3. If Don-vip doesn't respond, see if I can contact FOSSGIS to see if they have a way to get server access (which looks like it might work, given the response stoecker just posted).

We really do need to have a second person with server access (this is actually something that Stereo (a current OSMF board member) is worried about; he poked me awhile back about it anyway).

I can't remember who asked me if I wanted/had server access (either stoecker or Stereo). IIRC, I said I didn't think it was a good idea for me to (have it/ask for it) since I currently work as a contract worker at Kaart for Meta, and I don't want to deal with the compliance burden that would create.
Or the headache from the community (I don't know what issue they would come up with, but I want no part of it).

[stoecker]

Replying to taylor.smock:

We really do need to have a second person with server access (this is actually something that Stereo (a current OSMF board member) is worried about; he poked me awhile back about it anyway).

Yes, since Vincent seems no longer active I have to think about a solution here. While I'm much easier with many other admin rights (you probably have all I can think of :-), the SSH server access needs a lot of trust (recent xz issue is a good example why). So it takes time to find a solution, especially as I don't know people personally.

I can't remember who asked me if I wanted/had server access (either stoecker or Stereo). IIRC, I said I didn't think it was a good idea for me to (have it/ask for it) since I currently work as a contract worker at Kaart for Meta, and I don't want to deal with the compliance burden that would create.
Or the headache from the community (I don't know what issue they would come up with, but I want no part of it).

I assumed something like this after you didn't react too enthusiastic when I did a preliminary check with you.

I'm a stubborn older guy, so I deal with the community stuff fine. But I also learned that sometimes this needs discouraging fights and actually one of my reasons is also to keep that away from the active contributors: It's much easier to do changes when somebody else makes the big decisions (as long as that one listens to input). As long as I make the decisions that also means I'm responsible for the good or bad resulting from them.

Nevertheless I still think about having you as an emergency replacement ;-)

[taylor.smock]

Replying to stoecker:

Yes, since Vincent seems no longer active I have to think about a solution here. While I'm much easier with many other admin rights (you probably have all I can think of :-), the SSH server access needs a lot of trust (recent xz issue is a good example why). So it takes time to find a solution, especially as I don't know people personally.

Don-vip at least seems to read some of his email (see comment:69:ticket:16010).
I do agree that SSH server access needs a lot of trust (and I have told people who poke me for technical help in the past to never give remote access to someone over the internet).
Maybe one of the OSM system administrators would be a good "in case of emergency" option?

I'm a stubborn older guy, so I deal with the community stuff fine. But I also learned that sometimes this needs discouraging fights and actually one of my reasons is also to keep that away from the active contributors: It's much easier to do changes when somebody else makes the big decisions (as long as that one listens to input). As long as I make the decisions that also means I'm responsible for the good or bad resulting from them.

If it was just me, I wouldn't care. Unfortunately, I have to keep in mind that sometimes people get pressured into decisions by a small group of people. And I've had 3 or 4 different managers (at Meta) since 2020, so I can't trust that I will get one with the ability to wait and think over a controversy before taking action.
I might be a bit influenced by discussions with Bryan Housel (iD/Rapid) though.

Nevertheless I still think about having you as an emergency replacement ;-)

As an emergency replacement, I don't mind. I don't know how that would be set up though.

[stoecker]

As small note regarding your changes:

Using marktr() for the string is only necessary when it is initialized before the I18n setup (globals) or translation may change over runtime (stored instance class variable). For local variables and variables in classes which aren't stored you simply can call String x = tr("y") and then use x without tr().

[taylor.smock]

One of the things I've considered doing is looking into what it would take to allow i18n changes without restart; the big problem is static instances, so I'd rather do marktr for static variables.

But you were probably talking about reserved = marktr("reserved") in the PlatformHooks. Yeah, I should have just done reserved = tr("reserved");.

Our i18n system is very much non-standard. :)

[stoecker]

Our i18n system is very much non-standard. :)

On the contrary, our I18n is the OSS-Standard GNU gettext. It's only non-standard when you are used to the ugly Java style where nobody seems to have the balls to replace it with a hash based method. JOSM uses it's own lightweight implementation because GNU gettext for Java was not good (don't know if that changed).

Only non-standard is the file format which was designed to save space (the gettext .mo files each contain the English original, so you have these texts 20 times for 20 languages. Our format only has texts once, but this means files must always be updated together.

[GerdP]

Replying to taylor.smock:

I think I also need to revert the changes for toUnmodifiableMap.

Maybe. See #23748

[taylor.smock]

In 19133/josm:

See #23727, #23748: Re-enable Map.ofEntries for Utils#toUnmodifiableMap

This restores the behavior that was present prior to r19101 without reflection.

In r19100 and earlier, the NPE was caught and rethrown as a reflection exception.
At this point, we then returned a Collections.unmodifiableMap object.