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

images: Support symlinks to Tezi tarballs #13

Closed
wants to merge 1 commit into from

Conversation

matthijskooijman
Copy link
Contributor

The filename of the tarball is used to predict the name of the directory contained in the tarball. When a symlink is used with a different (simpler) name, the unpack would fail.

This resolves the symlink and looks at the filename of the actual tarball file.

This can be useful to allow pointing tcbuild.yaml to a symlink with a stable name, which points to a locally built tarball which changes name on every build. For example, see this patch that does this automatically for the yocto builds:

https://community.toradex.com/t/patch-create-non-versioned-symlink-to-tezi-tarball/21265

The filename of the tarball is used to predict the name of the directory
contained in the tarball. When a symlink is used with a different
(simpler) name, the unpack would fail.

This resolves the symlink and looks at the filename of the actual
tarball file.

This can be useful to allow pointing tcbuild.yaml to a symlink with
a stable name, which points to a locally built tarball which changes
name on every build. For example, see this patch that does this
automatically for the yocto builds:

    https://community.toradex.com/t/patch-create-non-versioned-symlink-to-tezi-tarball/21265

Signed-off-by: Matthijs Kooijman <matthijs@stdin.nl>
@lucas-akira
Copy link
Contributor

Hi @matthijskooijman ,

I think that the underlying issue is this part that you pointed out:

The filename of the tarball is used to predict the name of the directory contained in the tarball. When a symlink is used with a different (simpler) name, the unpack would fail.

This limitation is something on our current code, and I believe that allowing any .tar filename to be processed, independently of the directory name contained in it, should be a better and more general solution to this problem.

One way I see to solve it is to remove the 'Tezi' and 'teziimage' string checks, and after extracting the .tar contents check if extract_dir has only one element, and it's a directory.

  • If it's the case, then the contents are inside a subdir:
subdir_path = os.path.join(extract_dir, extract_dir_ls[0])
image_dir = subdir_path
  • Otherwise the contents are directly in extract_dir:
image_dir = extract_dir

Symlinks should be supported by this method as well.

What do you think? Can you adapt your PR to do this?

I may be overreaching a bit here, as I'm requesting that you do significant changes to your original request. I would understand if you're not willing to do this.

@matthijskooijman
Copy link
Contributor Author

One way I see to solve it is to remove the 'Tezi' and 'teziimage' string checks, and after extracting the .tar contents check if extract_dir has only one element, and it's a directory.

That is definitely the better solution to this. I just did not want to make too big changes (though I guess I failed that with some of my other PRs anyway).

What do you think? Can you adapt your PR to do this?

I'm willing to do this, but it might take me a while to find the time for it - I really have to make some actual progress with our project first before I spend more time on tools. I'll let you know if/when I start on this, if you happen to pick this up in the meantime let me know and we'll prevent double work :-)

@matthijskooijman
Copy link
Contributor Author

@lucas-akira I've implemented the more generic approach (including a more thorough restructuring of the code to make it more robust) in #15, so I'm closing this PR in favor of that one.

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

2 participants