Opened 9 years ago

Closed 9 years ago

#12481 closed defect (fixed)

Fix branch_current_hg()

Reported by: jdemeyer Owned by: jason
Priority: major Milestone: sage-5.0
Component: misc Keywords:
Cc: Merged in: sage-5.0.beta4
Authors: Jeroen Demeyer Reviewers: André Apitzsch
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by jdemeyer)

This is just horrible code (sage/misc/misc.py):

def branch_current_hg():
    """
    Return the current hg Mercurial branch name. If the branch is
    'main', which is the default branch, then just '' is returned.
    """
    try:
        s = os.popen('ls -l %s/devel/sage'%os.environ['SAGE_ROOT']).read()
    except IOError:
        # this happens when running sage under gdb on macs
        return 'gdb'
    if 'No such file or directory' in s:
        raise RuntimeError, "unable to determine branch?!"
    # do ls -l and look for a symlink, which `ls` represents by a '->'
    i = s.rfind('->')
    if i == -1:
        raise RuntimeError, "unable to determine branch?!"
    s = s[i+2:]
    i = s.find('-')
    if i == -1:
        return ''
    br = s[i+1:].strip()
    return br

For one, this clearly needs an "os.readlink()" instead of parsing "ls" output. The "gdb" is also broken, since the string "gdb" doesn't contain "->". Also the documentation is wrong: is the branch is "main", then "main" is returned.

Attachments (1)

12481.patch (1.7 KB) - added by jdemeyer 9 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:3 Changed 9 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Status changed from new to needs_review

comment:4 follow-up: Changed 9 years ago by aapitzsch

Could you use

raise RuntimeError("Unable to determine branch")

to have less work in the future (when we are moving to Python 3).

Changed 9 years ago by jdemeyer

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

Replying to aapitzsch:

Could you use

raise RuntimeError("Unable to determine branch")

to have less work in the future (when we are moving to Python 3).

Done.

comment:6 Changed 9 years ago by aapitzsch

  • Reviewers set to André Apitzsch
  • Status changed from needs_review to positive_review

Looks good.

comment:7 Changed 9 years ago by jdemeyer

  • Merged in set to sage-5.0.beta4
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.