Skip to content

Implement stdin stdout procedures addressing Issue #47.#62

Closed
greebie wants to merge 16 commits intomasterfrom
issue-47
Closed

Implement stdin stdout procedures addressing Issue #47.#62
greebie wants to merge 16 commits intomasterfrom
issue-47

Conversation

@greebie
Copy link
Copy Markdown
Collaborator

@greebie greebie commented Sep 19, 2018

GitHub issue(s):

#47

What does this Pull Request do?

Creates a more straightforward stdin stdout procedure call.

e.g. ./graphpass /path/to/input/file /path/to/output/outfile -{flags} will run graphpass on file and store it in outfile.

The stdin and stdout paths can appear anywhere in the expression (you can put {flags} or other options before, between or after the path calls. However, the order of stdin must come before stdout.

The -i or --input will override an stdin call.
The -i or --output will override a stdout call.

Both will have the same behavior as the above mentioned calls.

Graphpass will create an output/ directory in the call ./graphpass /path/to/input/file /path/to/output/outfile -{flags} but only if /path/to/ exists. Otherwise it will throw an error
with a message to create /path/to/.

Any non-flag arguments beyond stdin and stdout will be ignored for now (although graphpass will store them for future use).

How should this be tested?

A description of what steps someone could take to:

  • graphpass should accept the stdin / stdout procedure described above.
  • all tests should pass.
  • it should accept the case where stdout does not exist.
  • no filenames should continue to run the test graphml (currently cpp2.graph in /src/resources).

Additional Notes:

As of writing, I realize that i need to update the documentation in README. I will also check the case where there is no stdout declared.

Interested parties

@ianmilligan1 and @ruebot

Thanks in advance for your help with the Archives Unleashed Project!

- Current unit tests pass.
- Requires full testing.
- you can do this now ./graphpass {stdin} -{Flags} --{OPTION} {OPTION-VALUE} {stdout}
- or ./graphpass -{Flags} --{OPTION} {OPTION-VALUE} {stdin} {stdout]
The only constraint is that stdin comes before stdout.
- If stdout has a '\' at the end, graphpass will use stdin file name.
- Otherwise, stdin will use the filename indicated in stdout.

ex. "/Users/username/filepath/file.graphml /Users/username/OUT/" will put a "file.graphml" in /OUT/.
    "/Users/username/filepath/file.graphml /Users/username/OUT/outfile.graphml" will put "outfile.graphml" in /OUT/ instead.
@ruebot
Copy link
Copy Markdown
Member

ruebot commented Sep 19, 2018

Graphpass will create an output/ directory in the call

Why this? Can you explain the rationale here? It's my understanding that stdout should just be stdout. I don't understand the use case of forcing the creation of an output dir.


You also have commits in here from the previous PR.

@greebie
Copy link
Copy Markdown
Collaborator Author

greebie commented Sep 19, 2018

Graphpass will create an output/ directory in the call

Why this? Can you explain the rationale here? It's my understanding that stdout should just be stdout. I don't understand the use case of forcing the creation of an output dir.

It will only try to create the output directory if stdout does not exist. Otherwise it will just use stdout.

The way the program is written now, if no stdout exists, and there is no -g flag (to switch to .gexf) the user's original file will be over-written with the graphpassed update. That's why I included code to set the output directory to ./OUT/ if stdout does not exist.

It's possible to fail on no stdout instead, but I think it should be part of a different issue, given that the unit tests work based on the current approach.

You also have commits in here from the previous PR.

I do not understand github properly. Will fix.

(This is to confirm Travis fails are not due to existing compiled tests.)
If you do not clean, the qp and ana binaries will not be recompiled and will fail on run.
@greebie
Copy link
Copy Markdown
Collaborator Author

greebie commented Sep 19, 2018

Getting a seg fault in Travis. Not sure why because it builds fine on my system.

@ianmilligan1
Copy link
Copy Markdown
Member

I see that it builds fine on my Mac but fails on rho

i.e.

Makefile:27: recipe for target 'install' failed
make: [install] Segmentation fault (core dumped) (ignored)

@greebie
Copy link
Copy Markdown
Collaborator Author

greebie commented Sep 19, 2018

I see that it builds fine on my Mac but fails on rho

i.e.

Makefile:27: recipe for target 'install' failed
make: [install] Segmentation fault (core dumped) (ignored)

sigh That's what I thought. Going to try and force stdin stdout to see if that fixes things.

@ruebot
Copy link
Copy Markdown
Member

ruebot commented Sep 19, 2018

It will only try to create the output directory if stdout does not exist. Otherwise it will just use stdout.

That's not how stdout works. We shouldn't be clobbering an existing file if an option isn't given, nor creating a new directory.

I'm going to move on to other stuff for the rest of the day, and will be at York on Thursday and Friday. So, I won't get back to reviewing this until Monday at the earliest.


I do not understand github properly. Will fix.

It's not GitHub issue. It's the process I outlined last week in Slack. Please revisit that.

@greebie
Copy link
Copy Markdown
Collaborator Author

greebie commented Sep 19, 2018

Okay - the issue is that Mac has libgen.h compiled as part of gcc and Ubuntu does not. I will either add instructions to travis re: libgen.h or ignore create a function to get the directory name myself.

@greebie
Copy link
Copy Markdown
Collaborator Author

greebie commented Sep 20, 2018

That that Travis is hooked up, I am going to close this PR and re-open without the confusing commit problems.

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.

3 participants