Skip to content
This repository has been archived by the owner on May 14, 2022. It is now read-only.

Update exe only if our version is newer #41

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nxtn
Copy link
Member

@nxtn nxtn commented Mar 11, 2020

Fixes winsw/winsw#399

  • Our version is retrieved from project properties.
  • Current version is queried from the Windows file version.
  • Update exe only if our version is newer.

@nxtn nxtn changed the title Fix updater to not downgrade Update exe only if our version is newer Mar 11, 2020
@nxtn nxtn force-pushed the update branch 2 times, most recently from b01ad40 to ce790c6 Compare March 11, 2020 06:22
@nxtn
Copy link
Member Author

nxtn commented Mar 11, 2020

@oleg-nenashev Could you offer me some suggestions to deal with the warnings by SpotBugs?

@oleg-nenashev oleg-nenashev self-requested a review March 11, 2020 07:51
@oleg-nenashev oleg-nenashev self-assigned this Apr 6, 2020
@oleg-nenashev
Copy link
Member

Sorry, I missed that due to all other items on my plate. I will help to finish/integrate it as a part of the release for WinSW 2.7.0

@oleg-nenashev
Copy link
Member

FTR

08:27:37  [INFO] --- spotbugs-maven-plugin:3.1.12.2:check (spotbugs) @ windows-slave-installer ---
08:27:37  [INFO] BugInstance size is 2
08:27:37  [INFO] Error size is 0
08:27:37  [INFO] Total bugs: 2
08:27:37  [ERROR] Should org.jenkinsci.modules.windows_slave_installer.SlaveExeUpdater$1$1.invoke(File, VirtualChannel) return a zero length array rather than null? [org.jenkinsci.modules.windows_slave_installer.SlaveExeUpdater$1$1, org.jenkinsci.modules.windows_slave_installer.SlaveExeUpdater$1$1, org.jenkinsci.modules.windows_slave_installer.SlaveExeUpdater$1$1] At SlaveExeUpdater.java:[line 84]At SlaveExeUpdater.java:[line 88]At SlaveExeUpdater.java:[line 92] PZLA_PREFER_ZERO_LENGTH_ARRAYS
08:27:37  [ERROR] Class org.jenkinsci.modules.windows_slave_installer.SlaveExeUpdater$1$1 defines non-transient non-serializable instance field this$1 [org.jenkinsci.modules.windows_slave_installer.SlaveExeUpdater$1$1] In SlaveExeUpdater.java SE_BAD_FIELD
08:27:37  [INFO] 

Both issues should be fixed by switching to a copy-on-write array. Also it might be preferable to just use the VersionNumber class from https://github.com/jenkinsci/lib-version-number . It should be already available to the module from the Jenkins Core classpath

@nxtn nxtn force-pushed the update branch 2 times, most recently from 1e020aa to fb27ee7 Compare April 6, 2020 12:11
@nxtn
Copy link
Member Author

nxtn commented Jan 1, 2021

OK, OK, I got it.

This passes:

private static class Absolutize extends SlaveToMasterFileCallable<String> {
    private static final long serialVersionUID = 1L;
    public String invoke(File f, VirtualChannel channel) throws IOException {
        return f.getAbsolutePath();
    }
}
agentExe.act(new Absolutize());

This doesn't:

agentExe.act(new SlaveToMasterFileCallable<String>() {
    private static final long serialVersionUID = 1L;
    public String invoke(File f, VirtualChannel channel) throws IOException {
        return f.getAbsolutePath();
    }
});

Damn good.

@nxtn nxtn marked this pull request as ready for review January 1, 2021 13:25
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

@nxtn
Copy link
Member Author

nxtn commented Jan 1, 2021

  • There is a VersionNumber class which could help with version comparison

It'd be better if there is a constructor that accepts integers rather than strings.

  • It is possible to automate provisioning of the version through a filtered resource.

Will do.

@jglick jglick requested a review from oleg-nenashev May 3, 2021 16:51
@jglick
Copy link
Member

jglick commented May 3, 2021

@oleg-nenashev could you review & merge please? I would like to https://github.com/jenkinsci/jep/blob/master/jep/230/README.adoc#specification but I am blocked by the presence of nontrivial unmerged PRs.

@oleg-nenashev
Copy link
Member

In my queue, sorry

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

the .exe modifies itself at runtime, making itself incompatible after initial run
3 participants