Skip to content

binary distribution: relocate text files properly in relative binaries#13578

Merged
tgamblin merged 4 commits intoreleases/v0.13from
bugfix/relative-binaries-text-relocation
Nov 5, 2019
Merged

binary distribution: relocate text files properly in relative binaries#13578
tgamblin merged 4 commits intoreleases/v0.13from
bugfix/relative-binaries-text-relocation

Conversation

@becker33
Copy link
Copy Markdown
Member

@becker33 becker33 commented Nov 4, 2019

@tgamblin I think we need this for tomorrow.

Copy link
Copy Markdown
Member

@tgamblin tgamblin left a comment

Choose a reason for hiding this comment

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

@becker33: reviewed. Can you add some rationale to the PR and some comments to the text, in addition to addressing the questions below?

if not pat.search(data):
return
ndata = pat.sub(replace, data)
pat = re.compile(rb'(?<![\w\-_/])[\w\-_]*?%s([\w\-_/]*)' % old_dir.encode('utf-8'))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't believe that rb works in python 2. Does it have to be rb? Won't r work?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you document this to explain what the negative lookbehind does? If I understand this correctly, it's saying to only replace old_dir at the beginning of the path by ensuring that the string is not preceded by words, dashes, underscores, or a slash. Is that basically right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

r won't work in this context because we're operating on byte objects, so everything in re needs to be byte objects as well.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll need to use b and just escape my backslashes.

@tgamblin tgamblin merged commit 385e41d into releases/v0.13 Nov 5, 2019
tgamblin pushed a commit that referenced this pull request Nov 18, 2019
#13578)

* Make relative binaries relocate text files properly
* rb strings aren't valid in python 2
* move perl to new interface for setup_environment family methods
perl_bin_path = ':'.join(perl_bin_dirs)
spack_env.prepend_path('PATH', perl_bin_path)
run_env.prepend_path('PATH', perl_bin_path)
env.prepend_path('PATH', perl_bin_path)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@alalazo pointed me to this line as the reason for #13930

@tgamblin tgamblin deleted the bugfix/relative-binaries-text-relocation branch December 24, 2019 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants