Bug 510124

Comment 1 Ed Merks 2017-01-09 10:22:25 EST

Sorry, but there just isn't enough API and existing classes are often poorly implemented, e.g., why can't a JobManager work without a Workbench instance?

I'll mark this as an enhancement request and when (more likely if) I find time, I'll open Bugzillas for all the platform features that make bad implementation assumptions, e.g., a Workbench instance must exist, being a major common one.

Comment 2 Stefan Xenos 2017-01-09 10:54:04 EST

I wouldn't personally classify fixing API violations as an "enhancement"... but as long as we actually fix them, I'll try not to complain too much about the misclassification.

As far as I know, JobManager *can* exist without a workbench. Please file a bug if it can't. Bug 508982 related to ProgressManager, which is part of the workbench - so it's quite reasonable for it to assume that the workbench exists.

Comment 3 Eike Stepper 2017-01-09 11:02:58 EST

Hi Stefan,

Thanks for pointing out that the usage of internal classes is not optimal.

Please keep in mind that Oomph deploys itself into any IDE that it installs, including IDEs with platform versions that are out-of-maintenance for a long time already. How big are chances that bugzillas against these versions get fixed?

Comment 4 Mikaël Barbero 2017-01-09 11:31:39 EST

(In reply to Eike Stepper from comment #3)
> Hi Stefan,
> 
> Thanks for pointing out that the usage of internal classes is not optimal.
> 
> Please keep in mind that Oomph deploys itself into any IDE that it installs,
> including IDEs with platform versions that are out-of-maintenance for a long
> time already. How big are chances that bugzillas against these versions get
> fixed?

I know you will scream at me, but the only solution here is to have different plugins / codebase for older versions of Eclipse and the main stream. It's the counterpart on using internals.

Comment 5 Eike Stepper 2017-01-09 11:47:30 EST

But how would that address the problem that fixes for new bugzillas are not applied to old platform versions anymore?

Comment 6 Mikaël Barbero 2017-01-09 16:03:09 EST

It does not. 

I'm curious how did you workaround the fact the JobManager does not work without the workbench?

Comment 7 Eike Stepper 2017-01-10 00:06:25 EST

I believe that Ed was referring to this code:

      run(jobName, new ProgressLogRunnable()
      {
        public Set<String> run(ProgressLog log) throws Exception
        {
          final ProgressManager oldProgressProvider = ProgressManager.getInstance();
          ProgressLogProvider newProgressLogProvider = new ProgressLogProvider(progressPageLog, oldProgressProvider);

          // When the workbench isn't running org.eclipse.ui.internal.progress.new JobChangeAdapter()'s done(IJobChangeEvent) method one cleanup.
          JobChangeAdapter jobChangeListener = PlatformUI.isWorkbenchRunning() ? null : new JobChangeAdapter()
          {
            @Override
            public void done(IJobChangeEvent event)
            {
              Job job = event.getJob();
              for (JobInfo jobInfo : oldProgressProvider.getJobInfos(true))
              {
                if (jobInfo.getJob() == job)
                {
                  oldProgressProvider.removeJobInfo(jobInfo);
                  break;
                }
              }
            }
          };

          IJobManager jobManager = Job.getJobManager();
          jobManager.setProgressProvider(newProgressLogProvider);
          if (jobChangeListener != null)
          {
            jobManager.addJobChangeListener(jobChangeListener);
          }

          try
          {
            performer.perform(progressPageLog);
            return performer.getRestartReasons();
          }
          finally
          {
            jobManager.setProgressProvider(oldProgressProvider);
            if (jobChangeListener != null)
            {
              jobManager.removeJobChangeListener(jobChangeListener);
            }
          }
        }
      });

Comment 8 Stefan Xenos 2017-01-10 11:47:22 EST

Please correct me if I'm wrong, but it looks to me as though the entire point of that code is to cause this line to be executed every time a job is done and only if the workbench is not running:

oldProgressProvider.removeJobInfo(jobInfo);

Why is this necessary? Wouldn't the progress manager itself be unused if the workbench isn't running?

Comment 9 Ed Merks 2017-01-10 11:55:58 EST

We use the progress manager and the job manager.  The progress manager allows us to log what is being done by jobs (that are spawned by tasks being performed in the setup).  See how we're replacing it with something that delegates to the existing one?  The job listener ensures that job information is cleaned.  Given the job information is created regardless of whether there is a workbench, it makes sense it should be cleaned with the same disregard.

Comment 10 Ed Merks 2018-11-20 15:44:55 EST

My general experience is that getting the platform to add/change APIs is futile:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=538077

As such, unfortunate workarounds is the only possible way to make progress that would otherwise be blocked by people who do not have time, and set overly high standards for others to follow but that they don't actually follow themselves.

So this is a CANTFIX because fixing requires an army of cooperative people with copious spare time....