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

Implement Window 10 symlinks #109

Closed
serge-nikulin opened this issue Sep 15, 2017 · 15 comments
Closed

Implement Window 10 symlinks #109

serge-nikulin opened this issue Sep 15, 2017 · 15 comments
Assignees

Comments

@serge-nikulin
Copy link
Contributor

serge-nikulin commented Sep 15, 2017

Implement symlinks via built-in mklink console command that apparently has minimum restrictions.

@serge-nikulin
Copy link
Contributor Author

Please assign it to me

@dirk-thomas
Copy link
Contributor

Please take a look at the existing but stale PR #6 which already uses the mklink command.

@serge-nikulin
Copy link
Contributor Author

I see, thanks.
I’ll try nevertheless: it’s too much pain to do development without symlinks.
The fix will require changes in both ament_cmake and ament_tools.
Should I create two separate issues or this singe one will suffice?

@mikaelarguedas
Copy link
Contributor

you can use this single issue to track the feature addition and use Connects to ament/ament_cmake#109 in every PR you open to implement it. That will allow to both cross reference them on github and to connect them together on our waffle board

@dirk-thomas
Copy link
Contributor

The fix will require changes in both ament_cmake and ament_tools.

What do you expect to change in ament_tools? It already sets the CMake variable AMENT_CMAKE_SYMLINK_INSTALL when a symlink install is requested.

@serge-nikulin
Copy link
Contributor Author

helper.py:

def ament_symlink(source_path, destination_path):
    if os.name == 'nt':
        # Provide Windows 10 compatible symlink
        mklink = "mklink"
        command = [];
        command += ["cmd.exe", "/C", mklink]
        if os.path.isdir(source_path):
             command += ["/D"]
        command += [destination_path.replace('/', '\\')]
        command += [source_path]
        with subprocess.Popen(command, stderr=subprocess.PIPE) as p:
            output = p.stderr.read().decode("utf-8")
            p.wait()
            if output.startswith("Access is denied"):
                raise PermissionError("Permission denied: '%s' -> '%s'" % (source_path, destination_path))
            elif output.startswith("'%s' is not recognized" % mklink):
                raise NotImplementedError()
    else:
        os.symlink(source_path, destination_path)

@dirk-thomas
Copy link
Contributor

That makes sense to also affect the symlinks created by the tool. You should make sure that the logic doesn't rely on a specific locale but works for all users.

@serge-nikulin
Copy link
Contributor Author

I don't like that / to \ replacement but I don't know any standard way in Python to convert to native path.
CMake has it better with TO_NATIVE_PATH.

@serge-nikulin
Copy link
Contributor Author

@dirk-thomas,
ament-tools installation with -s switch fails in a curious way (see console log below):

1. No --symlink-install
src\ament\ament_tools\setup.py being executed in src\ament\ament_tools folder and successfully copies completion/ament-completion.* files to install/share/ament_tools/environment folder

2. --symlink-install present
an empty build\ament_tools\completion folder created and ament_tools\setup.py being executed in build\ament_tools folder and fails to copy copies completion/ament-completion.* files because build\ament_tools\completion folder is empty.

I wonder is it intended behavior?

+++ Installing 'ament_tools'
-- [ament] Deploying: C:\Users\serge\Documents\workdir\ros2_ws\install\share\ament_tools\package.xml
-- [ament] Deploying: C:\Users\serge\Documents\workdir\ros2_ws\install\share\ament_tools\environment\ament_prefix_path.bat
-- [ament] Deploying: C:\Users\serge\Documents\workdir\ros2_ws\install\share\ament_tools\environment\path.bat
-- [ament] Deploying: C:\Users\serge\Documents\workdir\ros2_ws\install\share\ament_tools\environment\pythonpath.bat
-- [ament] Deploying: C:\Users\serge\Documents\workdir\ros2_ws\install\share\ament_tools\local_setup.bat
==> 'C:\Users\serge\Documents\workdir\ros2_ws\build\ament_tools\ament_python__install.bat C:\Python\python.exe setup.py develop --prefix C:\Users\serge\Documents\workdir\ros2_ws\install --no-deps install_data --install-dir C:\Users\serge\Documents\workdir\ros2_ws\install' in 'C:\Users\serge\Documents\workdir\ros2_ws\build\ament_tools'
running develop
running egg_info
writing ament_tools.egg-info\PKG-INFO
writing dependency_links to ament_tools.egg-info\dependency_links.txt
writing entry points to ament_tools.egg-info\entry_points.txt
writing requirements to ament_tools.egg-info\requires.txt
writing top-level names to ament_tools.egg-info\top_level.txt
reading manifest file 'ament_tools.egg-info\SOURCES.txt'
writing manifest file 'ament_tools.egg-info\SOURCES.txt'
running build_ext
Creating c:\users\serge\documents\workdir\ros2_ws\install\lib\site-packages\ament-tools.egg-link (link to .)
ament-tools 0.0.0 is already the active version in easy-install.pth
Installing ament-script.py script to C:\Users\serge\Documents\workdir\ros2_ws\install/Scripts
Installing ament.exe script to C:\Users\serge\Documents\workdir\ros2_ws\install/Scripts

Installed c:\users\serge\documents\workdir\ros2_ws\build\ament_tools
running install_data
error: can't copy 'completion\ament-completion.bash': doesn't exist or not a regular file

<== Command 'C:\Users\serge\Documents\workdir\ros2_ws\build\ament_tools\ament_python__install.bat C:\Python\python.exe setup.py develop --prefix C:\Users\serge\Documents\workdir\ros2_ws\install --no-deps install_data --install-dir C:\Users\serge\Documents\workdir\ros2_ws\install' failed in 'C:\Users\serge\Documents\workdir\ros2_ws\build\ament_tools' with exit code '1'

@dirk-thomas
Copy link
Contributor

I wonder is it intended behavior?

Atm we can't use --symlink-install on Windows.

I am not sure if I understand your question but once that option can be used on Windows the intended behavior is obviously not a failure but that is performs the build and install using symlinks rather then copying files (as it already is on Linux and MacOS).

@mikaelarguedas
Copy link
Contributor

@serge-nikulin As we don't see a way to achieve --symlink-install on Windows without requiring admin permissions (a undesired workflow in our opinion) I'm going to close this.
Feel free to comment on the ticket if you have ideas of how to achieve this and we can reopen

@serge-nikulin
Copy link
Contributor Author

@mikaelarguedas yes, you can close it. I stuck with a Python3 bug (what I perceive as a bug) in Windows specific code. If they fix it I might take another attempt at this one.

@calvertdw
Copy link

Sad this isn't working. I am having to run in Administor mode anyway, as suggested by the Windows Development Setup tutorial.

It is probably worth right clicking on the shortcut, selecting properties, then advanced and checking the run as administrator box so that it always opens as administrator. Then you can right click it again and say pin to taskbar so it's easy to get to.

Is it possible to implement this anyway and assume administrator mode?

@serge-nikulin
Copy link
Contributor Author

@calvertdw , I believe it's a Python's bug. It does not implement a correct symlink command for Windows 10. In the Win10 command line, mklink works fine in user mode.

@calvertdw
Copy link

Is there a ticket with them for the issue?

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

No branches or pull requests

4 participants