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

Fix repository name component error w/ Docker 1.8 #94

Merged
merged 1 commit into from Aug 23, 2015

Conversation

mani-monaj
Copy link
Contributor

Apparently Docker 1.8 enforces the following regex for container names: [a-z0-9]+(?:[._-][a-z0-9]+)*. Therefore it does not allow repetitive _ characters for repository name components. This PR replaces all occurrences of __ with _ in empy templates to fix this issue. Currently pre-release script fails on Docker 1.8 with the following error:

repository name component must match \"[a-z0-9]+(?:[._-][a-z0-9]+)*\"

PS. This is the commit in Docker source tree which implements this feature. It must have been presented in Docker releases since v1.7. However I started getting this error after upgrading to Docker v1.8.

@tfoote
Copy link
Member

tfoote commented Aug 17, 2015

Thanks this does fix the error, but we used the double underscore as a unique value to avoid possible collisions with package names. @dirk-thomas do you have a suggestion of a new separator based on the new rules? It looks like we could use a .?

@dirk-thomas
Copy link
Member

A dot sounds good to me.

@mani-monaj
Copy link
Contributor Author

we used the double underscore as a unique value to avoid possible collisions with package names

Good to know.

It looks like we could use a .?

I'll test it and update this PR if everything works well.

@mani-monaj
Copy link
Contributor Author

@tfoote @dirk-thomas

It is quite straightforward to change all instances of __%s with .%s (e.g devel_build_and_install__%s_%s to devel_build_and_install.%s_%s). Is that good enough?

One other option is to use . instead of _ for all component names, but it probably needs manual editing of all template files.

- Comply with Docker's regex for repository name components
- Use dots instead of double underscores
@mani-monaj
Copy link
Contributor Author

I updated the PR. All double underscores have been replaced by a single dot.

@tfoote
Copy link
Member

tfoote commented Aug 22, 2015

That looks good. I'll try to find some time to test it this weekend.

@jack-oquin
Copy link

I tested Mani's docker_regexp_fix branch on bwi_common. My development system is Trusty, with docker 1.8.1 installed.

For 64-bit Trusty it seems to work, mostly. It did throw an exception at the end:

  • KeyError: "The cache has no package named 'ros-indigo-segbot'"

I doubt that has anything to do with this issue. The desired repository did get built and its tests ran successfully (although the test output is hard to find).

@jack-oquin
Copy link

I tried to test the same repository on Saucy, expecting to get a compile error because of the older yaml-cpp version. Instead, it failed to generate the script:

(venv)joq@iron:prerelease_job$ generate_prerelease_script.py   https://raw.githubusercontent.com/ros-infrastructure/ros_buildfarm_config/master/index.yaml   indigo default ubuntu saucy amd64   bwi_common   --level 1   --output-dir ./
Fetching buildfarm configuration...
Fetching rosdistro cache...
Evaluating job templates...
Traceback (most recent call last):
  File "/home/joq/ros_buildfarm_ws/venv/bin/generate_prerelease_script.py", line 5, in <module>
    pkg_resources.run_script('ros-buildfarm==0.2.1-master', 'generate_prerelease_script.py')
  File "/home/joq/ros_buildfarm_ws/venv/local/lib/python2.7/site-packages/pkg_resources.py", line 488, in run_script
    self.require(requires)[0].run_script(script_name, ns)
  File "/home/joq/ros_buildfarm_ws/venv/local/lib/python2.7/site-packages/pkg_resources.py", line 1354, in run_script
    execfile(script_filename, namespace, namespace)
  File "/home/joq/ros_buildfarm_ws/venv/lib/python2.7/site-packages/ros_buildfarm-0.2.1_master-py2.7.egg/EGG-INFO/scripts/generate_prerelease_script.py", line 263, in <module>
    sys.exit(main())
  File "/home/joq/ros_buildfarm_ws/venv/lib/python2.7/site-packages/ros_buildfarm-0.2.1_master-py2.7.egg/EGG-INFO/scripts/generate_prerelease_script.py", line 157, in main
    source_repository=source_repository)
  File "/home/joq/ros_buildfarm_ws/venv/local/lib/python2.7/site-packages/ros_buildfarm-0.2.1_master-py2.7.egg/ros_buildfarm/devel_job.py", line 237, in configure_devel_job
    ', '.join(sorted(build_file.targets[os_name].keys())))
ros_buildfarm.common.JobValidationError

Perhaps this problem is unrelated.

@dirk-thomas
Copy link
Member

Thank you @mani-monaj I can confirm that it fixes the problem with Docker 1.8.x.

dirk-thomas added a commit that referenced this pull request Aug 23, 2015
Fix repository name component error w/ Docker 1.8
@dirk-thomas dirk-thomas merged commit 97b41a3 into ros-infrastructure:master Aug 23, 2015
@jack-oquin
Copy link

When will this fix be propagated to user jobs? It's not there yet.

@mani-monaj
Copy link
Contributor Author

@dirk-thomas You are welcome.

@jack-oquin

I tried to test the same repository on Saucy, expecting to get a compile error because of the older yaml-cpp version. Instead, it failed to generate the script:

This is also a known issue and a patch has almost been approved for it. Please check #90 and #93.

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

Successfully merging this pull request may close these issues.

None yet

4 participants