Opened 4 years ago

Closed 4 years ago

#22570 closed defect (fixed)

Various fixes to (lib)GAP workspaces

Reported by: jdemeyer Owned by:
Priority: blocker Milestone: sage-7.6
Component: interfaces Keywords:
Cc: dimpase, vbraun Merged in:
Authors: Jeroen Demeyer Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: fb31078 (Commits, GitHub, GitLab) Commit: fb310787feb0896b5db5b06c606fb3d23bd239a5
Dependencies: Stopgaps:

Status badges

Description

  1. Factor out some common code for creating GAP and libGAP workspaces.
  1. Ensure that workspaces are written without race conditions (possible fix for #22536)
  1. Consider a workspace up-to-date if its timestamp equals the minimal timestamp (possible fix for #20421)

Change History (12)

comment:1 Changed 4 years ago by jdemeyer

  • Branch set to u/jdemeyer/various_fixes_to__lib_gap_workspaces

comment:2 Changed 4 years ago by jdemeyer

  • Commit set to 71e7692b28b863d616ec72eef57991bb7374a835
  • Status changed from new to needs_review

New commits:

71e7692Various fixes to (lib)GAP workspaces

comment:3 follow-up: Changed 4 years ago by dimpase

How do you handle a (not very likely) scenario of two processes trying to SaveWorkspace?

comment:4 in reply to: ↑ 3 Changed 4 years ago by jdemeyer

Replying to dimpase:

How do you handle a (not very likely) scenario of two processes trying to SaveWorkspace?

I use atomic_write() to write the workspaces. Therefore, there should be no race conditions (as long as the workspaces are written from Sage: direct calls from GAP can still do anything).

comment:5 follow-up: Changed 4 years ago by dimpase

By the way, one more reason for this ticket is that (now removed) sage -c "gap_reset_workspace()" in spkg-install of database_gap sometimes hanged the installation if kept there, whereas without this call one might have to run sage -c "gap_reset_workspace()" separately, as recently witnessed on #22576.

comment:6 Changed 4 years ago by dimpase

As your branch does not reintroduce sage -c "gap_reset_workspace()" in build/pkgs/database_gap/spkg-install, I wonder whether the problem mentioned in the previous comment would persist. (Or you really better put it back in?)

comment:7 Changed 4 years ago by jdemeyer

It's not the job of a package install script to do something with user files such as the GAP workspaces. In a multi-user installation, this should be done for every user and spkg-install cannot do that.

comment:8 in reply to: ↑ 5 Changed 4 years ago by jdemeyer

Replying to dimpase:

one might have to run sage -c "gap_reset_workspace()" separately, as recently witnessed on #22576.

I see that as an independent issue. Let's deal with that either on #22576 or on a new ticket.

comment:9 Changed 4 years ago by git

  • Commit changed from 71e7692b28b863d616ec72eef57991bb7374a835 to d23c8211db7ae695db3ea8e296572557c5320a03

Branch pushed to git repo; I updated commit sha1. New commits:

d23c821Drop dependency of database_gap on SAGERUNTIME

comment:10 Changed 4 years ago by git

  • Commit changed from d23c8211db7ae695db3ea8e296572557c5320a03 to fb310787feb0896b5db5b06c606fb3d23bd239a5

Branch pushed to git repo; I updated commit sha1. New commits:

fb31078Merge tag '7.6.rc0' into t/22570/various_fixes_to__lib_gap_workspaces

comment:11 Changed 4 years ago by vbraun

  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

comment:12 Changed 4 years ago by vbraun

  • Branch changed from u/jdemeyer/various_fixes_to__lib_gap_workspaces to fb310787feb0896b5db5b06c606fb3d23bd239a5
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.