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

Pid inclusion on setup #1731

Closed

Conversation

KOlofinmoyin
Copy link
Contributor

observer that the tmp/pid/server.pid keeps getting omitted on startup of a new repo/ pulling from origin master so included a rule in the gitignore to include the folder whenever there's a pull from master.

@nickcharlton
Copy link
Member

Did #1726 not help at all?

EWright212 and others added 2 commits August 5, 2020 15:38
…rver to be run locally file correctly added

Co-authored-by: KOlofinmoyin <[email protected]>
@EWright212
Copy link
Contributor

Did #1726 not help at all?

Unfortunately no, this new PR works for me as the file missing was tmp/pids/server.pid so we have included that one

Copy link
Collaborator

@pablobm pablobm 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 a vexing issue indeed. I've been looking into it a bit more deeply and this is what I think is going on.

At a0eeca6 (hereinafter "commit A"), @nickcharlton added the file /tmp/pids/.keep back in the repository. This file is necessary to avoid the folder /tmp/pids from being deleted. The reason it was being deleted is that Git doesn't know about folders, only about files. If it sees empty folders when moving across commits, it deletes them. Keeping a file (any file, even if empty) in a folder avoids this.

If you checkout the above commit (and just in case make sure that /tmp/pids exists) everything will work well.

If you move one commit "up" (eg: the commit next to A, 15d71a6, call it A+1), everything will work well.

However, if you move one commit "down" instead (eg: the commit before A, aecfe54, call it A-1), you won't be able to start the server because tmp/pids will have been deleted.

This is the chain of events:

  1. HEAD is at A (ie: we are at A).
  2. A has /tmp/pids/.keep.
  3. We checkout A-1.
  4. Git sees that /tmp/pids/.keep is removed, leaving /tmp/pids empty.
  5. Git removes /tmp/pids after emptying it.
  6. Now we can't run the dev server.

Note a different behaviour when moving between commits such that neither has /tmp/pids/.keep. For example:

  1. HEAD is at A-1.
  2. This doesn't have /tmp/pids/.keep.
  3. Ensure that /tmp/pids exists.
  4. Run the dev server. It should work.
  5. Checkout A-2 (df3969f)
  6. Folder /tmp/pids is still there!
  7. The dev server should work.

This is because Git only removes empty folders if they were emptied by Git itself. If they were empty before and after moving across commits, it lets them be.

So I think that the problem will be experienced by anyone who moves across affected commits, as a result of working with different versions of the codebase.

And my proposed solution is: do nothing. The codebase should be in the correct state now. Rebase your code to work on top of commits that do have /tmp/pids/.keep. Over time, the problem will disappear for we will not be using the affected code.

/spec/example_app/log/*
/spec/example_app/tmp/*
!/spec/example_app/log/.keep
!/spec/example_app/tmp/.keep
/spec/example_app/public/system
/spec/example_app/public/assets
/tags
/tmp/config
!/tmp/pids/server.pid
Copy link
Collaborator

Choose a reason for hiding this comment

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

We cannot allow /tmp/pids/server.pid to be checked into the repository. This file is different for each user, and for each instance of the server that a user runs. If a user were to clone it, with the file contents of another user, it would stop their own server from running.

@nickcharlton
Copy link
Member

Closing this as we concluded that whilst there's still an issue around this, we'll not take this approach.

Some of the problem being faced here was a messy clone, which was the origin of this particular 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

Successfully merging this pull request may close these issues.

4 participants