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

Do not update the updater separately #83

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

Conversation

frauzufall
Copy link
Member

The removed code does the following:

  • check if the updater can be updated
  • if yes, download updater and it's dependencies and install it, do not ask for approval
  • try to restart the updater in place
  • if that does not work, ask for restart of Fiji

This is why it should be removed:

  • if other components depend on the updater, this can break the updater because the other component will not get updated at the same time as the updater
  • the updater will be updated without the users approval

@ctrueden
Copy link
Member

The reason for the failing Travis build is because Travis CI recently changed the default environment from trusty to xenial. It seems the xvfb config doesn't work for xenial as written. As a quick fix, we can add dist: trusty to the top of .travis.yml. But it would be good to investigate why it doesn't work on xenial, and ideally update it to work, so that we aren't blindsided if/when trusty stops being supported.

Since Travis changed the default environment from trusty to xenial, xvfb does not have to be started in the before_script section. The service will get started automatically.
The removed code does the following:
  * check if the updater can be updated
  * if yes, download updater and it's dependencies and install it,
    do not ask for approval
  * try to restart the updater in place
  * if that does not work, ask for restart of Fiji

This is why it should be removed:
  * if other components depend on the updater, this can break the installation
    because the other component will not get updated at the same time as the updater
  * the updater will be updated without the users approval
@frauzufall frauzufall force-pushed the remove-updating-updater branch from a8adcdc to b75749e Compare August 20, 2019 08:45
@frauzufall
Copy link
Member Author

Thank you for pointing me to the right direction @ctrueden! I removed the lines in the travis config which are not needed any more in xenial and reverted the commit order so that there is no failing commit. Travis is happy now.

@ctrueden
Copy link
Member

I asked Johannes why this logic was introduced. Here is what he said:

The reason was dependency hell. If you don't update the updater first, then create a new class loader, then re-load the updater and run it, it really can break everything, especially when imagej-ui-swing is updated (and not necessarily the updater).

ctrueden pushed a commit that referenced this pull request Oct 20, 2024
The removed code does the following:
  * check if the updater can be updated
  * if yes, download updater and it's dependencies and install it,
    do not ask for approval
  * try to restart the updater in place
  * if that does not work, ask for restart of Fiji

This is why it should be removed:
  * if other components depend on the updater, this can break the installation
    because the other component will not get updated at the same time as the updater
  * the updater will be updated without the users approval

CTR: I asked Johannes why this logic was introduced. Here is what he said:

> The reason was dependency hell. If you don't update the updater first,
> then create a new class loader, then re-load the updater and run it,
> it really can break everything, especially when imagej-ui-swing is
> updated (and not necessarily the updater).

CTR: So really, there are things that can go wrong either way. In the
interest of simplicity, we remove the logic. If all hell breaks loose,
we can put it back, and deal with the fallout as best we can. But I
expect that removing this logic will fix more problems than it causes.

Closes #83.

Signed-off-by: Curtis Rueden <[email protected]>
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

Successfully merging this pull request may close these issues.

2 participants