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

Ignore missing files when compressing target directories #66

Closed
wants to merge 3 commits into from

Conversation

majk-p
Copy link

@majk-p majk-p commented Jun 9, 2021

This PR resolves #60

@majk-p
Copy link
Author

majk-p commented Jun 9, 2021

@djspiewak please have a look

Copy link
Collaborator

@djspiewak djspiewak left a comment

Choose a reason for hiding this comment

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

This is great! Thank you!

@djspiewak
Copy link
Collaborator

@majk-p Looks like some of the workflows need to be updated with the new flag!

@majk-p
Copy link
Author

majk-p commented Jun 15, 2021

Thanks @djspiewak! just regenerated the workflows, should work now 🤞

@djspiewak
Copy link
Collaborator

I think it's actually the tests that are the problem. Specifically:

  • non-existent-target
  • sbt-native-thin-client
  • check-and-regenerate

The easiest thing to do is probably go in and manually edit the ci.yml files in each to have the new flag on their tar invocations. You can check to make sure it's working correctly by running sbt scripted

@majk-p
Copy link
Author

majk-p commented Jun 15, 2021

Sorry, I should've tested it locally. The ci.yml files are now adjusted, sbt scripted passed with no issues. It should be ready now.

@majk-p
Copy link
Author

majk-p commented Jun 16, 2021

Looks like macos-latest failed due to tar: Option --ignore-failed-read is not supported. Doesn't macos go with regular gnu tar? is it just the workflow or does it break this way on desktop?

@djspiewak
Copy link
Collaborator

Doesn't macos go with regular gnu tar? is it just the workflow or does it break this way on desktop?

Seems to be that way on desktop as well. One way we can work around this is to just install our own tar.

@kubukoz
Copy link
Contributor

kubukoz commented Nov 30, 2021

I'm on macos and my tar is bsdtar, it does complain about the flag. Gnutar recognizes it.

edit: somehow I completely missed Daniel's comment saying basically the same thing 😂

@armanbilge
Copy link
Contributor

armanbilge commented Dec 31, 2021

Just saw this—I actually ran into this problem with feral and my solution was to mkdir -p all the targets instead, before the compressing step.

ThisBuild / githubWorkflowGeneratedUploadSteps ~= { steps =>
  val mkdirStep = steps.headOption match {
    case Some(WorkflowStep.Run(command :: _, _, _, _, _, _)) =>
      WorkflowStep.Run(
        commands = List(command.replace("tar cf targets.tar", "mkdir -p")),
        name = Some("Make target directories")
      )
    case _ => sys.error("Can't generate make target dirs workflow step")
  }
  mkdirStep +: steps
}

https://github.com/typelevel/feral/blob/579409b9e228854ba6ff3d8033cd5baa71793f4a/build.sbt#L45

Update: this fix is now automatically applied in sbt-typelevel.

benhutchison added a commit to benhutchison/gesture that referenced this pull request Feb 6, 2022
@mdedetrich
Copy link
Contributor

@armanbilge Should we aim to get this merged or is there some additional work that needs to be done that I am missing?

@armanbilge
Copy link
Contributor

@mdedetrich unfortunately the fix in this PR doesn't work. You should port my fix from sbt-typelevel that I mentioned above.

@mdedetrich
Copy link
Contributor

@armanbilge Thanks for the reply, I will close this PR then and we can reimplement the proper fix separately.

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.

“Compress target directories” step fails for Lagom projects.
5 participants